[amdgpu] Change LDS lowering default to hybrid

[JonChesterfield]

Summary

Postponed from D139433 until the bug fixed by D139874 could be resolved.

[JonChesterfield]

Internal CI number 792330 running at present.

[arsenm]

I would have expected a test update?

[arsenm]

I also wouldn’t call it a regalloc bug and point to to commit in the message

[JonChesterfield]

• Change to hybrid. Reintroduces code block deleted by git misfire

[JonChesterfield]

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

1307

This is latent because the module lds variable is always lowered to zero and the path through codegen coincidentally picks zero for it. It breaks on the "kernel" lowering strategy because that can reference variables that aren't at zero directly, and breaks on the "hybrid" lowering when it happens to pick the "kernel" strategy for a variable allocated by a kernel which does not use the "module" scope variable.

Still, relying on unspecified -> zero is bad. Planning to add a test case for this behaviour change (it won't show up in codegen that picks zero anyway, but should do for codegen where zero was not the right answer)

[JonChesterfield]

• update test, no longer emitting traps

[JonChesterfield]

Checked in the new test case (codegen for hybrid path, previously only checking IR) before this patch in order to highlight the s_trap -> working change. Will commit this as is to catch up with rocm and then change to absolute_symbols (as in D144221), patching trunk first so that rocm picks up the change without manual intervention.

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

 
 SDValue AMDGPUTargetLowering::LowerGlobalAddress(AMDGPUMachineFunction* MFI,
                                                  SDValue Op,
                                                  SelectionDAG &DAG) const {
 
   const DataLayout &DL = DAG.getDataLayout();
   GlobalAddressSDNode *G = cast<GlobalAddressSDNode>(Op);
   const GlobalValue *GV = G->getGlobal();
 
+  if (G->getAddressSpace() == AMDGPUAS::LOCAL_ADDRESS) {
+    if (!MFI->isModuleEntryFunction()) {
+      if (const GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {
+        if (AMDGPUMachineFunction::isKnownAddressLDSGlobal(*GVar)) {
+          unsigned Offset =
+              AMDGPUMachineFunction::calculateKnownAddressOfLDSGlobal(*GVar);
+          return DAG.getConstant(Offset, SDLoc(Op), Op.getValueType());
+        }
+      }
+    }
+  }
+
   if (G->getAddressSpace() == AMDGPUAS::LOCAL_ADDRESS ||
       G->getAddressSpace() == AMDGPUAS::REGION_ADDRESS) {
     if (!MFI->isModuleEntryFunction() &&
         !GV->getName().equals("llvm.amdgcn.module.lds")) {
       SDLoc DL(Op);
       const Function &Fn = DAG.getMachineFunction().getFunction();
       DiagnosticInfoUnsupported BadLDSDecl(
         Fn, "local memory global used by non-kernel function",

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp

cl::opt<bool> SuperAlignLDSGlobals(
     "amdgpu-super-align-lds-globals",
     cl::desc("Increase alignment of LDS if it is not on align boundary"),
     cl::init(true), cl::Hidden);
 
 enum class LoweringKind { module, table, kernel, hybrid };
 cl::opt<LoweringKind> LoweringKindLoc(
     "amdgpu-lower-module-lds-strategy",
     cl::desc("Specify lowering strategy for function LDS access:"), cl::Hidden,
-    cl::init(LoweringKind::module),
+    cl::init(LoweringKind::hybrid),
     cl::values(
         clEnumValN(LoweringKind::table, "table", "Lower via table lookup"),
         clEnumValN(LoweringKind::module, "module", "Lower via module struct"),
         clEnumValN(
             LoweringKind::kernel, "kernel",
             "Lower variables reachable from one kernel, otherwise abort"),
         clEnumValN(LoweringKind::hybrid, "hybrid",
                    "Lower via mixture of above strategies")));