[PATCH] D41715: AMDGPU: Process amdgpu.uniform on loads

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:134-135
+
+  // 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")) {
----------------
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


================
Comment at: lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:151-152
+    // a new one.
+    Value *Idx = Constant::getIntegerValue(
+                   Type::getInt32Ty(Ptr->getContext()), APInt(64, 0));
+    Instruction *GEP = GetElementPtrInst::Create(
----------------
ConstantInt::get is simpler


================
Comment at: lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:155
+             Ptr->getType()->getPointerElementType(), Ptr,
+             ArrayRef<Value*>(Idx), Twine(""), &I);
+    I.replaceUsesOfWith(Ptr, GEP);
----------------
You don't need the explicit Twine, you could also preserve the original name


================
Comment at: test/CodeGen/AMDGPU/amdgpu.uniform.ll:5
+; RUN: llc -march=amdgcn -mcpu=gfx900  -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=VIGFX9 %s
+
+; GCN-LABEL: {{^}}uniform_load:
----------------
This test should run the IR pass and check that


https://reviews.llvm.org/D41715