jhuber6 added a comment.
Thanks for the patch. I still think this is a silly feature to support, but
users will probably expect it. See comments.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10790
Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo;
- VarName = CGM.getMangledName(VD);
+ // We don't need to mangle the host side of declare target global
variables but we
+ // need to create offload entry that matches the device side which gets
mangled.
----------------
It might be easier to just mangle the original definition, that would reduce a
lot of churn here adding `origName` everywhere. Any reason that's not desirable?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10797
+ Out<<OrigName;
+ CGM.printPostfixForExternalizedDecl(Out, VD);
+ OffloadEntriesInfoManager.enterDeviceGlobalVarMangledName(OrigName,
Buffer);
----------------
`CGM.printPostfixForExternalizedDecl` should ideally give the same output on
the host and device, but it's somewhat limited since it just checks the file ID
and environment, which is technically possible to change. The kernels use
`getTargetEntryUniqueInfo`, which might make sense to re-use for this case.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7292
const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
+ // Make sure any variable with OpenMP declare target is visible to runtime
+ // except for those with hidden visibility
----------------
Formatting looks weird, did you do `git clang-format HEAD~1`?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9439
const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
+ // Make sure any variable with OpenMP declare target is visible to runtime
+ // except for those with hidden visibility
----------------
Just spitballing, is it possible to do this when we make the global instead?
================
Comment at: clang/test/OpenMP/declare_target_visibility_codegen.cpp:11-12
// HOST: @z = global i32 0
-// HOST-NOT: @.omp_offloading.entry.c
-// HOST-NOT: @.omp_offloading.entry.x
+// HOST: @.omp_offloading.entry.c__static__{{[0-9a-z]+_[0-9a-z]+}}
+// HOST: @.omp_offloading.entry.x__static__{{[0-9a-z]+_[0-9a-z]+}}
// HOST-NOT: @.omp_offloading.entry.y
----------------
If there are no updates between the host and device we can keep these static
without emitting an offloading entry.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129694/new/
https://reviews.llvm.org/D129694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
