-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HIP] Do not include the CUID module hash with the new driver #84332
Conversation
Summary: The new driver does not need this hash and it can lead to redefined symbol errors when the CUID hash isn't set.
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/84332.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index d02875c6a86d77..967319bdfc4571 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -916,7 +916,7 @@ void CodeGenModule::Release() {
llvm::ConstantArray::get(ATy, UsedArray), "__clang_gpu_used_external");
addCompilerUsedGlobal(GV);
}
- if (LangOpts.HIP) {
+ if (LangOpts.HIP && !getLangOpts().OffloadingNewDriver) {
// Emit a unique ID so that host and device binaries from the same
// compilation unit can be associated.
auto *GV = new llvm::GlobalVariable(
|
CUID is needed for device static variable to be accessible on host side. Since the driver does not know whether device static variables are accessed on host side, it should always enable CUID for HIP. |
Yeah that's what I was wondering about when I noticed that this CUID thing wasn't being set when @Meinersbur was testing on Windows. However, I don't think we need this module tag regardless. |
Oh! I think I remember what I did. I made the CUID hash generator thing make a fallback that just uses the file's |
As long as each generated .o has a unique CUID that is fine. Using inode as CUID has issue since users may compile the same file twice with different options to generate two different .o file (the case of rccl), then the CUID is not unique. |
Summary:
The new driver does not need this hash and it can lead to redefined
symbol errors when the CUID hash isn't set.