Skip to content

Conversation

@david-salinas
Copy link

… dynamic library is fully specified

@z1-cciauto
Copy link
Collaborator

@lamb-j
Copy link
Collaborator

lamb-j commented Oct 31, 2025

Should we move this PR upstream? Looks like the PSDB passed

@david-salinas david-salinas requested a review from omor1 October 31, 2025 16:32
} else if (isGPUBinHandleSymbol && (!isHidden) ) {
DefinedGPUBinHandleSymbols.insert(Name.str());
GPUBinHandleSymbols.erase(Name.str());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-      if (!isUndefined) {
+      if (!isUndefined && !isHidden) {
         if (isFatBinSymbol) {
           DefinedFatBinSymbols.insert(Name.str());
           FatBinSymbols.erase(Name.str());
-        } else if (isGPUBinHandleSymbol && (!isHidden) ) {
+        } else if (isGPUBinHandleSymbol) {
           DefinedGPUBinHandleSymbols.insert(Name.str());
           GPUBinHandleSymbols.erase(Name.str());
         }

I don't recall if I've specifically seen that __hip_fatbin symbols have the same issue, but it's probably best to avoid that issue as well if it crops up.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @omor1. You're right. Sam suggested we just remove the code to add these symbols if they are already defined. I've added a new commit. And the change is for both of the _hip symbols.

@z1-cciauto
Copy link
Collaborator

@david-salinas david-salinas force-pushed the amd/dev/dsalinas/swdev-561590 branch from 4bd0cf7 to c275297 Compare November 6, 2025 03:48
@z1-cciauto
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants