[CIR][CUDA][HIP] Set internal linkage for device variable shadows

[RiverDave]

Device variables (__device__, __constant__) now have internal linkage for their host-side shadow variables (non-RDC mode), matching OG behavior.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[koparasy]

I have some concerns with this. Some of the functionality you perform here should be done during lowering to LLVM IR (

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 1414 in 16cabe9

void LoweringPreparePass::buildCUDARegisterVars(cir::CIRBaseBuilderTy &builder,

). I believe the code that you have regarding internalize is proper here.

[koparasy]

I don't see this being used at CodeGen. We handle the "gpuBinaryHandle" during "lowering". Why do you think we need this here?

[RiverDave]

Good catch, this is merely an artifact from bringing the skeleton from OG, Will remove.

[koparasy]

Why do you need this? Does this exist in OG?

[koparasy]

I see you are mixing CUDA and HIP tests here. This is ok, but we had historically split them between CUDA/HIP directories.

[RiverDave]

Good Point, I'll make sure to split both things from now on.

[koparasy]

We prefer on not having a relative path here #include <cuda.h> :D. We give th path through the -I%S/../Inputs/ flag we pass to CC1.

[RiverDave]

I have some concerns with this. Some of the functionality you perform here should be done during lowering to LLVM IR (

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 1414 in 16cabe9

void LoweringPreparePass::buildCUDARegisterVars(cir::CIRBaseBuilderTy &builder,

). I believe the code that you have regarding internalize is proper here.

Okay, it took me some time. But I realize I misled with the initial title of this PR, Registration was recently handled in your PR (thanks for that!). The vector stored in the runtime deviceVars is metadata that we need to consume and utilize when making these registration calls. The problem I have with my PR is that we're not consuming that information at the loweringPrepare when we should make use of that.

See how OG handles the deviceVars:

clangir/clang/lib/CodeGen/CGCUDANV.cpp

Line 681 in 16cabe9

for (auto &&Info : DeviceVars) {

The equivalent we have to bookkeep globals in CIR is:

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 1466 in 16cabe9

for (auto &[deviceSideName, global] : cudaVarMap) {

If you look at the way we're currently handling the variables to be shadowed on the host in CIR:

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 131 in 16cabe9

llvm::StringMap<GlobalOp> cudaVarMap;

I believe we somehow need to preserve the information coming from VarInfo, specifically in DeviceVarFlags. Doing that allows us to give special handling to the different types of globals as seen in OG:

clangir/clang/lib/CodeGen/CGCUDANV.cpp

Line 687 in 16cabe9

switch (Info.Flags.getKind()) {

[RiverDave]

I have some concerns with this. Some of the functionality you perform here should be done during lowering to LLVM IR (

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 1414 in 16cabe9

void LoweringPreparePass::buildCUDARegisterVars(cir::CIRBaseBuilderTy &builder,

). I believe the code that you have regarding internalize is proper here.

Okay, it took me some time. But I realize I misled with the initial title of this PR, Registration was recently handled in your PR (thanks for that!). The vector stored in the runtime deviceVars is metadata that we need to consume and utilize when making these registration calls. The problem I have with my PR is that we're not consuming that information at the loweringPrepare when we should make use of that.

See how OG handles the deviceVars:

clangir/clang/lib/CodeGen/CGCUDANV.cpp

Line 681 in 16cabe9

for (auto &&Info : DeviceVars) {

The equivalent we have to bookkeep globals in CIR is:

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 1466 in 16cabe9

for (auto &[deviceSideName, global] : cudaVarMap) {

If you look at the way we're currently handling the variables to be shadowed on the host in CIR:

clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Line 131 in 16cabe9

llvm::StringMap<GlobalOp> cudaVarMap;

I believe we somehow need to preserve the information coming from VarInfo, specifically in DeviceVarFlags. Doing that allows us to give special handling to the different types of globals as seen in OG:

clangir/clang/lib/CodeGen/CGCUDANV.cpp

Line 687 in 16cabe9

switch (Info.Flags.getKind()) {

The way I see it, we have two paths:

• Utilizing a richer data structure to preserve those flags and consume that information in loweringPrepare
• Preserving those flags through attributes attached to Global Ops, although the implementation would take longer.

Let me know what you think