AMDGPU: Process amdgpu.uniform on loads

[mareko]

Summary

Optimizations passes can remove GEP with offset=0 and amdgpu.uniform,
but they are less likely to drop amdgpu.uniform from a load.

This is the only way to have uniform loads without GEP or when GEP is
eliminated.

[arsenm]

lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp

134–135

I'm not sure I understand the problem this is solving. Why is it dropped specifically if it is 0? If the offset is constant the GEP is uniform iff the base pointer is uniform

151–152

ConstantInt::get is simpler

155

You don't need the explicit Twine, you could also preserve the original name

test/CodeGen/AMDGPU/amdgpu.uniform.ll

5

This test should run the IR pass and check that

[mareko]

lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp

134–135

If the address is in VGPRs, the offset is 0, so we need to set amdgpu.uniform when there is no GEP.

[mareko]

lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp

155

How to do it without Twine and how to preserve the original name? Thanks.

test/CodeGen/AMDGPU/amdgpu.uniform.ll

5

Do you mean running the AnnotateUniformValues pass instead of llc? It looks like the llc test does the job too.

[nhaehnle]

lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp

143–146

This makes no sense to me. amdgpu.uniform is about whether the value could differ between threads in a wave. How often the value is used further downstream is irrelevant for that.

155

Just put "" instead of Twine("").

Not sure about preserving the original name here -- we're not transforming an instruction here, we're adding an intermediate instruction.

test/CodeGen/AMDGPU/amdgpu.uniform.ll

5

Running opt -amdgpu-annotate-uniform is potentially more stable against random changes in unrelated passes.

In this particular case though, I think using llc is justified, because we do need to test that the uniform annotation actually survives through to codegen.

[mareko]

I've abandoned this patch for now. Please re-open if you think this patch is useful.

lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp

void AMDGPUAnnotateUniformValues::visitBranchInst(BranchInst &I) {
   if (!DA->isUniform(Cond))
     return;
 
   setUniformMetadata(I.getParent()->getTerminator());
 }
 
 void AMDGPUAnnotateUniformValues::visitLoadInst(LoadInst &I) {
   Value *Ptr = I.getPointerOperand();
+
+  // This is true if loads have amdgpu.uniform on them (as opposed to GEP
+  // which might be dropped by InstCombine if the offset is 0).
+  if (I.getMetadata("amdgpu.uniform")) {
+    if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Ptr)) {
+      // If GEP is uniform, we don't need to do anything.
+      if (GEP->getMetadata("amdgpu.uniform"))
+        return;
+
+      // Set Uniform on the GEP if it only has 1 use.
+      if (GEP->hasOneUse()) {
+        setUniformMetadata(GEP);
+        return;
+      }
+    }
+
+    // There is no GEP or the GEP has multiple uses. We'll have to create
+    // a new one.
+    Value *Idx = Constant::getIntegerValue(
+                   Type::getInt32Ty(Ptr->getContext()), APInt(64, 0));
+    Instruction *GEP = GetElementPtrInst::Create(
+             Ptr->getType()->getPointerElementType(), Ptr,
+             ArrayRef<Value*>(Idx), Twine(""), &I);
+    I.replaceUsesOfWith(Ptr, GEP);
+    setUniformMetadata(GEP);
+    return;
+  }
+
   if (!DA->isUniform(Ptr))
     return;
   auto isGlobalLoad = [&](LoadInst &Load)->bool {
     return Load.getPointerAddressSpace() == AMDGPUASI.GLOBAL_ADDRESS;
   };
   // We're tracking up to the Function boundaries
   // We cannot go beyond because of FunctionPass restrictions
   // Thus we can ensure that memory not clobbered for memory

test/CodeGen/AMDGPU/amdgpu.uniform.ll

+; RUN: llc -march=amdgcn -mcpu=tahiti  -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=SICI -check-prefix=SI %s
+; RUN: llc -march=amdgcn -mcpu=bonaire -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=SICI %s
+; RUN: llc -march=amdgcn -mcpu=tonga   -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=VIGFX9 %s
+; RUN: llc -march=amdgcn -mcpu=gfx900  -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=VIGFX9 %s
+
+; GCN-LABEL: {{^}}uniform_load:
+; GCN: v_mov_b32_e32 v1, 0
+; GCN-NEXT: v_readfirstlane_b32 s0, v0
+; GCN-NEXT: v_readfirstlane_b32 s1, v1
+; SI-NEXT: s_nop
+; GCN-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x0
+define amdgpu_vs i32 @uniform_load(i32 %p) #0 {
+  %p64 = zext i32 %p to i64
+  %ptr = inttoptr i64 %p64 to i32 addrspace(2)*
+  %r = load i32, i32 addrspace(2)* %ptr, !amdgpu.uniform !0
+  ret i32 %r
+}
+
+; GCN-LABEL: {{^}}gep_uniform_load:
+; GCN: v_mov_b32_e32 v1, 0
+; GCN-NEXT: v_readfirstlane_b32 s0, v0
+; GCN-NEXT: v_readfirstlane_b32 s1, v1
+; SI-NEXT: s_nop
+; SICI-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x1
+; VIGFX9-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x4
+define amdgpu_vs i32 @gep_uniform_load(i32 %p) #0 {
+  %p64 = zext i32 %p to i64
+  %ptr = inttoptr i64 %p64 to i32 addrspace(2)*
+  %gep = getelementptr i32, i32 addrspace(2)* %ptr, i32 1
+  %r = load i32, i32 addrspace(2)* %gep, !amdgpu.uniform !0
+  ret i32 %r
+}
+
+; GCN-LABEL: {{^}}uniform_gep_load:
+; GCN: v_mov_b32_e32 v1, 0
+; GCN-NEXT: v_readfirstlane_b32 s0, v0
+; GCN-NEXT: v_readfirstlane_b32 s1, v1
+; SI-NEXT: s_nop
+; SICI-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x1
+; VIGFX9-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x4
+define amdgpu_vs i32 @uniform_gep_load(i32 %p) #0 {
+  %p64 = zext i32 %p to i64
+  %ptr = inttoptr i64 %p64 to i32 addrspace(2)*
+  %gep = getelementptr i32, i32 addrspace(2)* %ptr, i32 1, !amdgpu.uniform !0
+  %r = load i32, i32 addrspace(2)* %gep
+  ret i32 %r
+}
+
+; GCN-LABEL: {{^}}uniform_gep_uniform_load:
+; GCN: v_mov_b32_e32 v1, 0
+; GCN-NEXT: v_readfirstlane_b32 s0, v0
+; GCN-NEXT: v_readfirstlane_b32 s1, v1
+; SI-NEXT: s_nop
+; SICI-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x1
+; VIGFX9-NEXT: s_load_dword s{{[0-9]}}, s[0:1], 0x4
+define amdgpu_vs i32 @uniform_gep_uniform_load(i32 %p) #0 {
+  %p64 = zext i32 %p to i64
+  %ptr = inttoptr i64 %p64 to i32 addrspace(2)*
+  %gep = getelementptr i32, i32 addrspace(2)* %ptr, i32 1, !amdgpu.uniform !0
+  %r = load i32, i32 addrspace(2)* %gep, !amdgpu.uniform !0
+  ret i32 %r
+}
+
+attributes #0 = { nounwind }
+
+!0 = !{}