AMDGPU/SI: Don't promote alloca to vector for atomic load/store

[cfang]

Summary

Don't promote alloca to vector for atomic load/store

[arsenm]

lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp

327–328

You can use isSimple which checks volatile and atomic

test/CodeGen/AMDGPU/vector-alloca-atomic.ll

3

You shouldn't need this

[cfang]

Combine !isAtomic and !isVolatile checks as isSimple

[cfang]

test/CodeGen/AMDGPU/vector-alloca-atomic.ll

3

If I remove the line "target datalayout = "A5"", I will get the following errors:

chfang@shanghai:~/bug/llvm/build/bin$ ./opt -S -mtriple=amdgcn---amdgiz -amdgpu-promote-alloca -sroa -instcombine < ../../test/CodeGen/AMDGPU/vector-alloca-atomic.ll | ./FileCheck -check-prefix=OPT ../../test/CodeGen/AMDGPU/vector-alloca-atomic.ll
Allocation instruction pointer not in the stack address space!

%alloca = alloca [3 x i32], addrspace(5)

Allocation instruction pointer not in the stack address space!

%alloca = alloca [3 x i32], addrspace(5)

Allocation instruction pointer not in the stack address space!

%alloca = alloca [3 x i32], addrspace(5)

./opt: -: error: input module is broken!
FileCheck error: '-' is empty.

[cfang]

What do you think of the test?
What should we do if we don't add the following line?
target datalayout = "A5"

Thanks;

[cfang]

Update the test:

1. Remove the -amdgiz from the triple since it is no longer necessary;
2. Add "-data-layout=A5" to explicitly specify the data layout for address space 5 for alloca
   • and to remove the "target:" line for the same data layout purpose.

[cfang]

ping

[arsenm]

LGTM with the check lines enhanced

test/CodeGen/AMDGPU/vector-alloca-atomic.ll

30

These check lines should include the addrspace of the pointers used

51

Ditto

lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp

 
 // Not an instruction handled below to turn into a vector.
 //
 // TODO: Check isTriviallyVectorizable for calls and handle other
 // instructions.
 static bool canVectorizeInst(Instruction *Inst, User *User) {
   switch (Inst->getOpcode()) {
   case Instruction::Load: {
     LoadInst *LI = cast<LoadInst>(Inst);
     // Currently only handle the case where the Pointer Operand is a GEP so check for that case.
-    return isa<GetElementPtrInst>(LI->getPointerOperand()) && !LI->isVolatile();
+    return isa<GetElementPtrInst>(LI->getPointerOperand()) && LI->isSimple();
   }
   case Instruction::BitCast:
   case Instruction::AddrSpaceCast:
     return true;
   case Instruction::Store: {
     // Must be the stored pointer operand, not a stored value, plus
     // since it should be canonical form, the User should be a GEP.
     StoreInst *SI = cast<StoreInst>(Inst);
-    return (SI->getPointerOperand() == User) && isa<GetElementPtrInst>(User) && !SI->isVolatile();
+    return (SI->getPointerOperand() == User) && isa<GetElementPtrInst>(User) && SI->isSimple();
   }
   default:
     return false;
   }
 }
 
 static bool tryPromoteAllocaToVector(AllocaInst *Alloca, AMDGPUAS AS) {
 
   if (DisablePromoteAllocaToVector) {
     DEBUG(dbgs() << "  Promotion alloca to vector is disabled\n");

test/CodeGen/AMDGPU/vector-alloca-atomic.ll

+; RUN: opt -S -mtriple=amdgcn---amdgiz -amdgpu-promote-alloca -sroa -instcombine < %s | FileCheck -check-prefix=OPT %s
+target datalayout = "A5"
+
+
+; Show that what the alloca promotion pass will do for non-atomic load/store.
+
+; OPT-LABEL: @vector_alloca_not_atomic(
+;
+; OPT: extractelement <3 x i32> <i32 0, i32 1, i32 2>, i64 %index
+define amdgpu_kernel void @vector_alloca_not_atomic(i32 addrspace(1)* %out, i64 %index) {
+entry:
+  %alloca = alloca [3 x i32], addrspace(5)
+  %a0 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 0
+  %a1 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 1
+  %a2 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 2
+  store i32 0, i32 addrspace(5)* %a0
+  store i32 1, i32 addrspace(5)* %a1
+  store i32 2, i32 addrspace(5)* %a2
+  %tmp = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i64 0, i64 %index
+  %data = load i32, i32 addrspace(5)* %tmp
+  store i32 %data, i32 addrspace(1)* %out
+  ret void
+}
+
+; OPT-LABEL: @vector_alloca_atomic_read(
+;
+; OPT: alloca [3 x i32]
+; OPT: store i32 0
+; OPT: store i32 1
+; OPT: store i32 2
+; OPT: load atomic i32
+define amdgpu_kernel void @vector_alloca_atomic_read(i32 addrspace(1)* %out, i64 %index) {
+entry:
+  %alloca = alloca [3 x i32], addrspace(5)
+  %a0 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 0
+  %a1 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 1
+  %a2 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 2
+  store i32 0, i32 addrspace(5)* %a0
+  store i32 1, i32 addrspace(5)* %a1
+  store i32 2, i32 addrspace(5)* %a2
+  %tmp = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i64 0, i64 %index
+  %data = load atomic i32, i32 addrspace(5)* %tmp acquire, align 4
+  store i32 %data, i32 addrspace(1)* %out
+  ret void
+}
+
+; OPT-LABEL: @vector_alloca_atomic_write(
+;
+; OPT: alloca [3 x i32]
+; OPT: store atomic i32 0
+; OPT: store atomic i32 1
+; OPT: store atomic i32 2
+; OPT: load i32
+define amdgpu_kernel void @vector_alloca_atomic_write(i32 addrspace(1)* %out, i64 %index) {
+entry:
+  %alloca = alloca [3 x i32], addrspace(5)
+  %a0 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 0
+  %a1 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 1
+  %a2 = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i32 0, i32 2
+  store atomic i32 0, i32 addrspace(5)* %a0 release, align 4
+  store atomic i32 1, i32 addrspace(5)* %a1 release, align 4
+  store atomic i32 2, i32 addrspace(5)* %a2  release, align 4
+  %tmp = getelementptr [3 x i32], [3 x i32] addrspace(5)* %alloca, i64 0, i64 %index
+  %data = load i32, i32 addrspace(5)* %tmp
+  store i32 %data, i32 addrspace(1)* %out
+  ret void
+}