ssquare08 added inline comments.
================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10797 + Out<<OrigName; + CGM.printPostfixForExternalizedDecl(Out, VD); + OffloadEntriesInfoManager.enterDeviceGlobalVarMangledName(OrigName, Buffer); ---------------- jhuber6 wrote: > ssquare08 wrote: > > jhuber6 wrote: > > > `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. > > That was the point I had raised in one of the Clang meeting but someone had > > mentioned that kernels names are created on the host side and the device > > side reads the information though the Host IR. Seems like kernels name > > could also run into mismatch issue for some corner cases then? > So the problem here is that the host and device need to agree on what the > name is so that we can register the correct variable. The CUDA / HIP > toolchains solved this by either performing a mangling that is stable between > the host and device, or by having the driver generate a random hash that gets > used on both. OpenMP instead solves this by writing the variable to the host > IR first and then reading it on the device to see what the name needs to be. > Since we have that dependency we can use any mangling we want, though it's > still best for it to be somewhat stable unless we want tests to change every > time we run them. It probably won't hurt anything to just use > `printPostfixForExternalizedDecl` but it's not as strong of a mangling as > what we can do with the OpenMP method since it needs to be common between the > host and device. > `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. This has been changed as suggested. 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
