[PATCH] D35146: AMDGPU : Widen extending scalar loads to 32-bits

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Needs tests



================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:448
+bool AMDGPUCodeGenPrepare::visitLoadInst(LoadInst &I) {
+  bool Changed = false;
+
----------------
There's no point to having this since you never set it to anything else


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:453
+
+  if (!VT && needsPromotionToI32(I.getType()) && DA->isUniform(&I) &&
+	  I.getPointerAddressSpace() == AMDGPUASI.CONSTANT_ADDRESS) {
----------------
Ideally this would handle vectors like <2 x i8>


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:454
+  if (!VT && needsPromotionToI32(I.getType()) && DA->isUniform(&I) &&
+	  I.getPointerAddressSpace() == AMDGPUASI.CONSTANT_ADDRESS) {
+    IRBuilder<> Builder(&I);
----------------
The address space check should be first


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:458
+
+    Type *I32Ty = getI32Ty(Builder, I.getType());
+    Type *PT = PointerType::get(I32Ty, I.getPointerAddressSpace());
----------------
The builder already has a getInt32Ty


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:460
+    Type *PT = PointerType::get(I32Ty, I.getPointerAddressSpace());
+    Value *BitCast= Builder.CreateBitCast(I.getOperand(0), PT);
+    Value *WidenLoad = Builder.CreateLoad(BitCast, PT);
----------------
I.getPointerOperand


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:468
+  }
+
+  return Changed;
----------------
This should also skip volatile loads


Repository:
  rL LLVM

https://reviews.llvm.org/D35146