[amdgpu] Always, instead of mostly, remove unused LDS symbols

[JonChesterfield]

Summary

Currently LDS variables are removed by the lower module pass
if they have a use which is caught by the replace with struct control flow.
This makes tests brittle to changes to that control flow which induces
noise when trying to improve lowering. Some tests already check that
variables are removed, while others checked that they are not removed.

LDS variables are not (currently) externally accessible, and if that
changes the machinery which makes them externally accessible will look
like a use. This change therefore breaks no applications.

[jhuber6]

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp

247–253

Nit. formatting.

[rampitec]

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp

248

It can be a single if and no braces around eraseFromParent. A little more readable.

[JonChesterfield]

• remove braces

[JonChesterfield]

I like braces as they make git merge less error prone. Let me know if you want them removed from the for block as well.

[JonChesterfield]

• guess in favour of removing braces

[JonChesterfield]

Guessing in the other direction, please let me know if you want the braces put back in for the for.

[rampitec]

LGTM

[foad]

In D133028#3774623, @JonChesterfield wrote:

Guessing in the other direction, please let me know if you want the braces put back in for the for.

The LLVM rules are complicated -- too complicated in my opinion. In AMDGPU code we usually omit braces if the thing inside them would be a single line.

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp

for (Function &F : M.functions()) {
             M, KernelUsedVariables, SGV, LDSVarsToConstantGEP, [&F](Use &U) {
               Instruction *I = dyn_cast<Instruction>(U.getUser());
               return I && I->getFunction() == &F;
             });
         Changed = true;
       }
     }
 
+    for (auto &GV : make_early_inc_range(M.globals())) {
+      if (AMDGPU::isLDSVariableToLower(GV) && GV.use_empty())
+        GV.eraseFromParent();
+    }
+    
     return Changed;
   }
 
 private:
   // Increase the alignment of LDS globals if necessary to maximise the chance
   // that we can use aligned LDS instructions to access them.
   static bool superAlignLDSGlobals(Module &M) {
     const DataLayout &DL = M.getDataLayout();
     bool Changed = false;
     if (!SuperAlignLDSGlobals) {

llvm/test/CodeGen/AMDGPU/lower-kernel-lds-super-align.ll

 ; RUN: opt -S -mtriple=amdgcn-- -amdgpu-lower-module-lds --amdgpu-super-align-lds-globals=true < %s | FileCheck --check-prefixes=CHECK,SUPER-ALIGN_ON %s
 ; RUN: opt -S -mtriple=amdgcn-- -passes=amdgpu-lower-module-lds --amdgpu-super-align-lds-globals=true < %s | FileCheck --check-prefixes=CHECK,SUPER-ALIGN_ON %s
 ; RUN: opt -S -mtriple=amdgcn-- -amdgpu-lower-module-lds --amdgpu-super-align-lds-globals=false < %s | FileCheck --check-prefixes=CHECK,SUPER-ALIGN_OFF %s
 ; RUN: opt -S -mtriple=amdgcn-- -passes=amdgpu-lower-module-lds --amdgpu-super-align-lds-globals=false < %s | FileCheck --check-prefixes=CHECK,SUPER-ALIGN_OFF %s
 
 ; CHECK: %llvm.amdgcn.kernel.k1.lds.t = type { [32 x i8] }
 ; CHECK: %llvm.amdgcn.kernel.k2.lds.t = type { i16, [2 x i8], i16 }
 ; CHECK: %llvm.amdgcn.kernel.k3.lds.t = type { [32 x i64], [32 x i32] }
 ; CHECK: %llvm.amdgcn.kernel.k4.lds.t = type { [2 x i32 addrspace(3)*] }
 
 ; SUPER-ALIGN_ON: @lds.unused = addrspace(3) global i32 undef, align 4
 ; SUPER-ALIGN_OFF: @lds.unused = addrspace(3) global i32 undef, align 2
 @lds.unused = addrspace(3) global i32 undef, align 2
 
[e-mail] = appending global [1 x i8*] [i8* addrspacecast (i32 addrspace(3)* @lds.unused to i8*)], section "llvm.metadata"
+
 ; CHECK-NOT: @lds.1
 @lds.1 = internal unnamed_addr addrspace(3) global [32 x i8] undef, align 1
 
 ; SUPER-ALIGN_ON: @llvm.amdgcn.kernel.k1.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k1.lds.t undef, align 16
 ; SUPER-ALIGN_OFF: @llvm.amdgcn.kernel.k1.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k1.lds.t undef, align 1
 
 ; CHECK: @llvm.amdgcn.kernel.k2.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k2.lds.t undef, align 4
 ; SUPER-ALIGN_ON:  @llvm.amdgcn.kernel.k3.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k3.lds.t undef, align 16

llvm/test/CodeGen/AMDGPU/lower-kernel-lds.ll

 ; RUN: opt -S -mtriple=amdgcn-- -amdgpu-lower-module-lds < %s | FileCheck %s
 ; RUN: opt -S -mtriple=amdgcn-- -passes=amdgpu-lower-module-lds < %s | FileCheck %s
 
 @lds.size.1.align.1 = internal unnamed_addr addrspace(3) global [1 x i8] undef, align 1
 @lds.size.2.align.2 = internal unnamed_addr addrspace(3) global [2 x i8] undef, align 2
 @lds.size.4.align.4 = internal unnamed_addr addrspace(3) global [4 x i8] undef, align 4
[e-mail].8 = internal unnamed_addr addrspace(3) global [8 x i8] undef, align 8
 @lds.size.16.align.16 = internal unnamed_addr addrspace(3) global [16 x i8] undef, align 16
 
 ; CHECK: %llvm.amdgcn.kernel.k0.lds.t = type { [16 x i8], [4 x i8], [2 x i8], [1 x i8] }
 ; CHECK: %llvm.amdgcn.kernel.k1.lds.t = type { [16 x i8], [4 x i8], [2 x i8] }
 
 ;.
-; CHECK: @lds.size.8.align.8 = internal unnamed_addr addrspace(3) global [8 x i8] undef, align 8
 ; CHECK: @lds.k2 = addrspace(3) global [1 x i8] undef, align 1
 ; CHECK: @llvm.amdgcn.kernel.k0.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k0.lds.t undef, align 16
 ; CHECK: @llvm.amdgcn.kernel.k1.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k1.lds.t undef, align 16
 ;.
 define amdgpu_kernel void @k0() {
 ; CHECK-LABEL: @k0(
 ; CHECK-NEXT:    %lds.size.1.align.1.bc = bitcast [1 x i8] addrspace(3)* getelementptr inbounds (%llvm.amdgcn.kernel.k0.lds.t, %llvm.amdgcn.kernel.k0.lds.t addrspace(3)* @llvm.amdgcn.kernel.k0.lds, i32 0, i32 3) to i8 addrspace(3)*
 ; CHECK-NEXT:    store i8 1, i8 addrspace(3)* %lds.size.1.align.1.bc, align 2