Let llvm.invariant.group.barrier accepts pointer to any address space

[yaxunl]

Summary

llvm.invariant.group.barrier may accept pointers to arbitrary address space.

This patch let it accept pointers to i8 in any address space and returns
pointer to i8 in the same address space.

Corresponding clang change: https://reviews.llvm.org/D40062

[rampitec]

lib/Analysis/CaptureTracking.cpp

247

Can we mark it as "NoCapture<0>" instead?

[yaxunl]

lib/Analysis/CaptureTracking.cpp

247

line 243 also checks I->getType()->isVoidTy(), but this intrinsic returns a pointer. So even if we make it NoCapture, it still does not work.

[rampitec]

lib/Analysis/CaptureTracking.cpp

247

Checking for a specific intrinsic seems error-prone. What if the pointer returned by the call is further used in the caller?

[rampitec]

lib/Analysis/CaptureTracking.cpp

247

Maybe it is possible to add a check for VoidTy with the condition, that value is unused if returned from non-void?

[yaxunl]

lib/Analysis/CaptureTracking.cpp

247

Good idea. Will do.

[yaxunl]

Revised by Stas' comments.

[rampitec]

LGTM

[arsenm]

lib/Analysis/CaptureTracking.cpp

244

I don't understand the I->use_empty() check, or how this is related to the address space change

[arsenm]

test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll

2

Why is it necessary to add this? The datalayout is implied by the explicit triple

[yaxunl]

lib/Analysis/CaptureTracking.cpp

244

llvm.invariant.group.barrier only accepts pointer in default addr space. As such, the alloca has to be addrspacecasted then passed to llvm.invariant.group.barrier. In AMDGPUPromoteAlloca pass, addrspacecast is checked through CaptureTracking. The addrspacecast inst is passed to llvm.invariant.group.barrier, CaptureTracking thinks the intrinsic captures the addrspacecast inst because the intrinsic does not have void return type, which is incorrect. If the intrinsic only reads mem, does not throw, even if it returns a pointer, but if the returned pointer is not used, it still does not capture the pointer. So after this is fixed, CaptureTracking will identify the addrspacecast as not captured, then AMDGPUPromoteAlloca will identify the alloca as not captured, and then it can be promoted. Otherwise the alloca will not be promoted.

test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll

2

Removing this will cause error:

llc: <stdin>:13:31: error: address space must match datalayout

%tmp = alloca i32, align 4, addrspace(5)

[arsenm]

This is a workaround for invariant.group.barrier not having a mangled type. That should probably be fixed instead. The others already do, so it doesn't make sense that group.barrier does not

test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll

2

This sounds like a bug. This should be coming from the datalayout

[Prazek]

Firstly, are you sure that the invariant.group.barrier is the problem here? I didn't hear that it is used anywhere besides devirtualization, that is not turned on by default.
Here is my old path that was never reviewed:
https://reviews.llvm.org/D32673

As you can see, in order to check if the pointer passed to the barrier is captured, we need to check if all uses of the pointer returned by the barrier is not captured.

[yaxunl]

In D39973#924419, @Prazek wrote:

Firstly, are you sure that the invariant.group.barrier is the problem here? I didn't hear that it is used anywhere besides devirtualization, that is not turned on by default.
Here is my old path that was never reviewed:
https://reviews.llvm.org/D32673

As you can see, in order to check if the pointer passed to the barrier is captured, we need to check if all uses of the pointer returned by the barrier is not captured.

In D39973#924419, @Prazek wrote:

Firstly, are you sure that the invariant.group.barrier is the problem here? I didn't hear that it is used anywhere besides devirtualization, that is not turned on by default.
Here is my old path that was never reviewed:
https://reviews.llvm.org/D32673

As you can see, in order to check if the pointer passed to the barrier is captured, we need to check if all uses of the pointer returned by the barrier is not captured.

I am trying to fix a lit test failure related to invariant.group.barrier. There are two issues:

1. as Matt pointed out, invariant.group.barrier should accept pointer to any address space as parameter, since alloca address space may not be 0, e.g. for amdgcn---amdgiz triple, the pointer argument of invariant.group.barrier is in addr space 5.

2. CaptureTracking does not handle invariant.group.barrier correctly, which is what your patch and this patch tries to fix. Since you already have a patch for that, I would drop my fix and focus on the first issue. However as Stas suggested, it may be better to make the fix more generic instead of do special handling for invariant.group.barrier, e.g. is it OK to make it a generic checking for all functions?

if the func does not read mem
if the func does not throw
if the func returns nothing, or the returned pointer is not captured

then pointer argument is not captured

[yaxunl]

test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll

2

The error msg comes from LLParser::ParseAlloc.

llc load and parse IR before setting target triple and datalayout for the module.

When llc parses IR, it has to use the datalayout defined by the IR itself. If the IR does not define datalayout, the default layout created from empty string is used.

Currently LLParser does not allow overriding the datalayout of the IR by a given datalayout.

If we want to change this, we need to add an API to LLParser to allow datalayout to be set before parsing IR, and change llc to set datalayout before parsing IR. I think this is better left for another patch.

[yaxunl]

Make llvm.invariant.group.barrier mangled by Matt's comments.

[Prazek]

LGTM

lib/Analysis/CaptureTracking.cpp

while (!Worklist.empty()) {
     switch (I->getOpcode()) {
     case Instruction::Call:
     case Instruction::Invoke: {
       CallSite CS(I);
       // Not captured if the callee is readonly, doesn't return a copy through
       // its return value and doesn't unwind (a readonly function can leak bits
       // by throwing an exception or not depending on the input value).
       if (CS.onlyReadsMemory() && CS.doesNotThrow() && I->getType()->isVoidTy())
         break;
 
+      // Intrinsic function invariant_group_barrier does not capture pointer.
+      if (isa<IntrinsicInst>(I) && cast<IntrinsicInst>(I)->getIntrinsicID() ==
+                                       Intrinsic::invariant_group_barrier) {
+        break;
+      }
+
       // Volatile operations effectively capture the memory location that they
       // load and store to.
       if (auto *MI = dyn_cast<MemIntrinsic>(I))
         if (MI->isVolatile())
           if (Tracker->captured(U))
             return;
 
       // Not captured if only passed via 'nocapture' arguments.  Note that

test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll

-; RUN:  llc -amdgpu-scalarize-global-loads=false  -march=amdgcn -mattr=+promote-alloca -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+; RUN:  llc -amdgpu-scalarize-global-loads=false  -march=amdgcn -mtriple=amdgcn---amdgiz -mattr=+promote-alloca -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+target datalayout = "A5"
 
-declare {}* @llvm.invariant.start.p0i8(i64, i8* nocapture) #0
-declare void @llvm.invariant.end.p0i8({}*, i64, i8* nocapture) #0
+declare {}* @llvm.invariant.start.p5i8(i64, i8 addrspace(5)* nocapture) #0
+declare void @llvm.invariant.end.p5i8({}*, i64, i8 addrspace(5)* nocapture) #0
 declare i8* @llvm.invariant.group.barrier(i8*) #1
 
 ; GCN-LABEL: {{^}}use_invariant_promotable_lds:
 ; GCN: buffer_load_dword
 ; GCN: ds_write_b32
 define amdgpu_kernel void @use_invariant_promotable_lds(i32 addrspace(1)* %arg) #2 {
 bb:
-  %tmp = alloca i32, align 4
-  %tmp1 = bitcast i32* %tmp to i8*
+  %tmp = alloca i32, align 4, addrspace(5)
+  %tmp1 = bitcast i32 addrspace(5)* %tmp to i8 addrspace(5)*
   %tmp2 = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 1
   %tmp3 = load i32, i32 addrspace(1)* %tmp2
-  store i32 %tmp3, i32* %tmp
-  %tmp4 = call {}* @llvm.invariant.start.p0i8(i64 4, i8* %tmp1) #0
-  call void @llvm.invariant.end.p0i8({}* %tmp4, i64 4, i8* %tmp1) #0
-  %tmp5 = call i8* @llvm.invariant.group.barrier(i8* %tmp1) #1
+  store i32 %tmp3, i32 addrspace(5)* %tmp
+  %tmp4 = call {}* @llvm.invariant.start.p5i8(i64 4, i8 addrspace(5)* %tmp1) #0
+  call void @llvm.invariant.end.p5i8({}* %tmp4, i64 4, i8 addrspace(5)* %tmp1) #0
+  %tmp5 = addrspacecast i8 addrspace(5)* %tmp1 to i8*
+  %tmp6 = call i8* @llvm.invariant.group.barrier(i8* %tmp5) #1
   ret void
 }
 
 attributes #0 = { argmemonly nounwind }
 attributes #1 = { nounwind readnone }
 attributes #2 = { nounwind }