[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added a comment. In D129694#3718225 , @ssquare08 wrote: > Yes, that is correct. My question is, is it okay to mangle the host and the > device side independently using `getTargetEntryUniqueInfo`? The reason I am > asking is because you had expre

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added a comment. In D129694#3717208 , @jhuber6 wrote: > In D129694#3717166 , @ssquare08 > wrote: > >> The OpenMP kernel names you mentioned are also generated separately by the >> host and the device.

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added a comment. In D129694#3717166 , @ssquare08 wrote: > The OpenMP kernel names you mentioned are also generated separately by the > host and the device. Would you be okay generating declare target mangle names > separately by host and device

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 marked an inline comment as not done. ssquare08 added inline comments. 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 r

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added a comment. In D129694#3717008 , @ssquare08 wrote: > In D129694#3708297 , @jhuber6 wrote: > >> In D129694#3708214 , @ssquare08 >> wrote: >> >>> If that's t

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added a comment. In D129694#3708297 , @jhuber6 wrote: > In D129694#3708214 , @ssquare08 > wrote: > >> If that's the preference I can make changes as suggested. You mentioned CUDA >> and HIP mangle the

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added a comment. In D129694#3708214 , @ssquare08 wrote: > If that's the preference I can make changes as suggested. You mentioned CUDA > and HIP mangle the declaration directly. To me it looks like they mangle it > on host and device separately.

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added inline comments. 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 visibi

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added a comment. In D129694#3685207 , @jhuber6 wrote: > I still think we shouldn't bother making all the noise containing the > original name. Just mangle it and treat it like every other declare target > variable without introducing any extra

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added a comment. I still think we shouldn't bother making all the noise containing the original name. Just mangle it and treat it like every other declare target variable without introducing any extra complexity. These symbols never should've been emitted in the first place so I'm not c

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10797 +Out< ssquare08 wrote: > > jhuber6 wrote: > > > `CGM.printPostfixForExternalizedDecl` should ideally give the same output > > > on the host and device, but it's somewhat limited sinc

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added inline comments. Comment at: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp:61 // DEVICE-DAG: @G1 = {{.*}}global i32 0, align 4 -// DEVICE-DAG: @_ZL2G2 = internal {{.*}}global i32 0, align 4 +// DEVICE-DAG: @_ZL2G2 = {{.*}}global i32 0, align 4 /

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 updated this revision to Diff 447901. ssquare08 added a comment. Herald added a project: OpenMP. Herald added a subscriber: openmp-commits. Adding a test and fixing This adds a new runtime test and also address some comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added inline comments. Comment at: clang/test/OpenMP/target_update_messages.cpp:17 -static int y; -#pragma omp declare target(y) - -void yyy() { -#pragma omp target update to(y) // expected-error {{the host cannot update a declare target variable that is not externally

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jdoerfert added inline comments. Comment at: clang/test/OpenMP/target_update_messages.cpp:17 -static int y; -#pragma omp declare target(y) - -void yyy() { -#pragma omp target update to(y) // expected-error {{the host cannot update a declare target variable that is not external

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3284-3286 // Hidden or internal symbols on the device are not externally visible. We // should not attempt to register them by creating an offloading entry. if (auto *GV = dyn_c

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 added inline 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 b

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

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; -V

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

ssquare08 created this revision. ssquare08 added a reviewer: jhuber6. Herald added subscribers: mattd, asavonic, guansong, yaxunl. Herald added a project: All. ssquare08 requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald ad