[PATCH] D39973: Fix CaptureTracking for llvm.invariant.group.barrier

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl added inline comments.


================
Comment at: lib/Analysis/CaptureTracking.cpp:244
+      if (CS.onlyReadsMemory() && CS.doesNotThrow() &&
+          (I->getType()->isVoidTy() || I->use_empty()))
         break;
----------------
arsenm wrote:
> I don't understand the I->use_empty() check, or how this is related to the address space change
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.


================
Comment at: test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll:2
+; 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"
 
----------------
arsenm wrote:
> Why is it necessary to add this? The datalayout is implied by the explicit triple
Removing this will cause error:

llc: <stdin>:13:31: error: address space must match datalayout
  %tmp = alloca i32, align 4, addrspace(5)



https://reviews.llvm.org/D39973