[amdgpu] Error, instead of miscompile, anonymous kernels using lds

[JonChesterfield]

Summary

The association between kernel and struct is done by symbol name.
This doesn't work robustly for anonymous kernels as shown by the modified
test case.

An alternative association between function and struct can be constructed
if necessary, probably though metadata, but on the basis that we currently
miscompile anonymous kernels and that they are difficult to construct from
application code and difficult to call from the runtime, this patch makes
it a fatal error for now.

[JonChesterfield]

The other option is to generate a name for the anonymous kernels from within the lds pass but that's a gross mixing of concerns. D127052 suggested using Mangler::getNameWithPrefix, but that's a local state thing so an instance created in the lowering pass won't necessarily hand out the same names as an instance created in the backend. So we could use that to name the functions here but we'd have to actually setName them.

[arsenm]

Needs test that hits the error

[JonChesterfield]

I like the test idea but a credible guess at how to write one is not working. Can you see the mistake in this? not --crash opt < %s has a couple of existing examples in tree.

; RUN: not --crash opt -S -mtriple=amdgcn-- -amdgpu-lower-module-lds < %s | FileCheck %s
; RUN: not --crash opt -S -mtriple=amdgcn-- -passes=amdgpu-lower-module-lds < %s | FileCheck %s

@var1 = addrspace(3) global i32 undef, align 8

; CHECK: LLVM ERROR: Anonymous kernels cannot use LDS variables
define amdgpu_kernel void @0() {
  %val0 = load i32, i32 addrspace(3)* @var1
  %val1 = add i32 %val0, 4
  store i32 %val1, i32 addrspace(3)* @var1
  ret void
}

Fails with exit code: 2 and the stack trace, though it also prints that error exactly as specified by the check line

[JonChesterfield]

• add not working test case

[JonChesterfield]

• drop align from test

[arsenm]

llvm/test/CodeGen/AMDGPU/lds-reject-anonymous-kernels.ll

3

Needs 2>&1 to redirect the error to stdout

3

to stdin

[JonChesterfield]

• redirect stderr

llvm/test/CodeGen/AMDGPU/lds-reject-anonymous-kernels.ll

3

great spot, thanks!

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp

for (Function &F : M.functions()) {
       // Only lower compute kernels' LDS.
       if (!AMDGPU::isKernel(F.getCallingConv()))
         continue;
 
       std::vector<GlobalVariable *> KernelUsedVariables =
           AMDGPU::findLDSVariablesToLower(M, &F);
 
       if (!KernelUsedVariables.empty()) {
+        // The association between kernel function and LDS struct is done by
+        // symbol name, which only works if the function in question has a name
+        // This is not expected to be a problem in practice as kernels are
+        // called by name making anonymous ones (which are named by the backend)
+        // difficult to use. This does mean that llvm test cases need
+        // to name the kernels.
+        if (!F.hasName()) {
+          report_fatal_error("Anonymous kernels cannot use LDS variables");
+        }
+
         std::string VarName =
             (Twine("llvm.amdgcn.kernel.") + F.getName() + ".lds").str();
         GlobalVariable *SGV;
         DenseMap<GlobalVariable *, Constant *> LDSVarsToConstantGEP;
         std::tie(SGV, LDSVarsToConstantGEP) =
             createLDSVariableReplacement(M, VarName, KernelUsedVariables);
 
         removeFromUsedLists(M, KernelUsedVariables);

llvm/test/CodeGen/AMDGPU/lds-reject-anonymous-kernels.ll

+; RUN: not --crash opt -S -mtriple=amdgcn-- -amdgpu-lower-module-lds < %s 2>&1 | FileCheck %s
+; RUN: not --crash opt -S -mtriple=amdgcn-- -passes=amdgpu-lower-module-lds < %s 2>&1 | FileCheck %s
+
+@var1 = addrspace(3) global i32 undef, align 8
+
+; CHECK: LLVM ERROR: Anonymous kernels cannot use LDS variables
+define amdgpu_kernel void @0() {
+  %val0 = load i32, i32 addrspace(3)* @var1
+  %val1 = add i32 %val0, 4
+  store i32 %val1, i32 addrspace(3)* @var1
+  ret void
+}

llvm/test/CodeGen/AMDGPU/lower-kernel-and-module-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
 @lds.size.8.align.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.module.lds.t = type { [8 x i8], [1 x i8] }
 ; CHECK: %llvm.amdgcn.kernel.k0.lds.t = type { [16 x i8], [4 x i8], [2 x i8] }
 ; CHECK: %llvm.amdgcn.kernel.k1.lds.t = type { [16 x i8], [4 x i8], [2 x i8] }
-; CHECK: %llvm.amdgcn.kernel..lds.t = type { [2 x i8] }
-; CHECK: %llvm.amdgcn.kernel..lds.t.0 = type { [4 x i8] }
+; CHECK: %llvm.amdgcn.kernel.k2.lds.t = type { [2 x i8] }
+; CHECK: %llvm.amdgcn.kernel.k3.lds.t = type { [4 x i8] }
 
 ;.
 ; CHECK: @llvm.amdgcn.module.lds = internal addrspace(3) global %llvm.amdgcn.module.lds.t undef, align 8
 ; CHECK: @llvm.compiler.used = appending global [1 x i8*] [i8* addrspacecast (i8 addrspace(3)* getelementptr inbounds (%llvm.amdgcn.module.lds.t, %llvm.amdgcn.module.lds.t addrspace(3)* @llvm.amdgcn.module.lds, i32 0, i32 0, i32 0) to i8*)], section "llvm.metadata"
 ; 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
-; CHECK: @llvm.amdgcn.kernel..lds = internal addrspace(3) global %llvm.amdgcn.kernel..lds.t undef, align 2
-; CHECK: @llvm.amdgcn.kernel..lds.1 = internal addrspace(3) global %llvm.amdgcn.kernel..lds.t.0 undef, align 4
+; CHECK: @llvm.amdgcn.kernel.k2.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k2.lds.t undef, align 2
+; CHECK: @llvm.amdgcn.kernel.k3.lds = internal addrspace(3) global %llvm.amdgcn.kernel.k3.lds.t undef, align 4
 ;.
 define amdgpu_kernel void @k0() #0 {
 ; CHECK-LABEL: @k0(
 ; CHECK-NEXT:    %lds.size.1.align.1.bc = bitcast [1 x i8] addrspace(3)* getelementptr inbounds (%llvm.amdgcn.module.lds.t, %llvm.amdgcn.module.lds.t addrspace(3)* @llvm.amdgcn.module.lds, i32 0, i32 1) to i8 addrspace(3)*
 ; CHECK-NEXT:    store i8 1, i8 addrspace(3)* %lds.size.1.align.1.bc, align 8
 ; CHECK-NEXT:    %lds.size.2.align.2.bc = bitcast [2 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 2) to i8 addrspace(3)*
 ; CHECK-NEXT:    store i8 2, i8 addrspace(3)* %lds.size.2.align.2.bc, align 4
 ; CHECK-NEXT:    %lds.size.4.align.4.bc = bitcast [4 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 1) to i8 addrspace(3)*
;
   store i8 4, i8 addrspace(3)* %lds.size.4.align.4.bc, align 4
 
   %lds.size.16.align.16.bc = bitcast [16 x i8] addrspace(3)* @lds.size.16.align.16 to i8 addrspace(3)*
   store i8 16, i8 addrspace(3)* %lds.size.16.align.16.bc, align 16
 
   ret void
 }
 
-define amdgpu_kernel void @0() #0 {
-; CHECK-LABEL: @0(
-; CHECK-NEXT:    %lds.size.2.align.2.bc = bitcast [2 x i8] addrspace(3)* getelementptr inbounds (%llvm.amdgcn.kernel..lds.t, %llvm.amdgcn.kernel..lds.t addrspace(3)* @llvm.amdgcn.kernel..lds, i32 0, i32 0) to i8 addrspace(3)*
+define amdgpu_kernel void @k2() #0 {
+; CHECK-LABEL: @k2(
+; CHECK-NEXT:    %lds.size.2.align.2.bc = bitcast [2 x i8] addrspace(3)* getelementptr inbounds (%llvm.amdgcn.kernel.k2.lds.t, %llvm.amdgcn.kernel.k2.lds.t addrspace(3)* @llvm.amdgcn.kernel.k2.lds, i32 0, i32 0) to i8 addrspace(3)*
 ; CHECK-NEXT:    store i8 2, i8 addrspace(3)* %lds.size.2.align.2.bc, align 2
 ; CHECK-NEXT:    ret void
 ;
   %lds.size.2.align.2.bc = bitcast [2 x i8] addrspace(3)* @lds.size.2.align.2 to i8 addrspace(3)*
   store i8 2, i8 addrspace(3)* %lds.size.2.align.2.bc, align 2
 
   ret void
 }
 
-define amdgpu_kernel void @1() #0 {
-; CHECK-LABEL: @1(
-; CHECK-NEXT:    %lds.size.4.align.4.bc = bitcast [4 x i8] addrspace(3)* getelementptr inbounds (%llvm.amdgcn.kernel..lds.t.0, %llvm.amdgcn.kernel..lds.t.0 addrspace(3)* @llvm.amdgcn.kernel..lds.1, i32 0, i32 0) to i8 addrspace(3)*
+define amdgpu_kernel void @k3() #0 {
+; CHECK-LABEL: @k3(
+; CHECK-NEXT:    %lds.size.4.align.4.bc = bitcast [4 x i8] addrspace(3)* getelementptr inbounds (%llvm.amdgcn.kernel.k3.lds.t, %llvm.amdgcn.kernel.k3.lds.t addrspace(3)* @llvm.amdgcn.kernel.k3.lds, i32 0, i32 0) to i8 addrspace(3)*
 ; CHECK-NEXT:    store i8 4, i8 addrspace(3)* %lds.size.4.align.4.bc, align 4
 ; CHECK-NEXT:    ret void
 ;
   %lds.size.4.align.4.bc = bitcast [4 x i8] addrspace(3)* @lds.size.4.align.4 to i8 addrspace(3)*
   store i8 4, i8 addrspace(3)* %lds.size.4.align.4.bc, align 4
 
   ret void
 }