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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:455
+  if (I.getPointerAddressSpace() == AMDGPUASI.CONSTANT_ADDRESS &&
+      !I.isVolatile() && TySize < 32 && DA->isUniform(&I) && Align >= 4) {
+    IRBuilder<> Builder(&I);
----------------
Move align check before DA. It would also be better to move the datalayout alignment check into a helper function checked here


================
Comment at: test/CodeGen/AMDGPU/widen_extending_scalar_loads.ll:5
+
+; FUNC-LABEL: {{^}}constant_load_i1:
+; GCN: load i1
----------------
Don't use FUNC, you don't have this as a check prefix


================
Comment at: test/CodeGen/AMDGPU/widen_extending_scalar_loads.ll:24-27
+; GCN: bitcast
+; GCN-NEXT: load
+; GCN-NEXT: trunc
+; GCN-NEXT: store
----------------
This isn't checking the relevant parts


================
Comment at: test/CodeGen/AMDGPU/widen_extending_scalar_loads.ll:133
+
+; FUNC-LABEL: {{^}}constant_load_half:
+; GCN: load half
----------------
_f16 is the naming convention


================
Comment at: test/CodeGen/AMDGPU/widen_extending_scalar_loads.ll:145
+; GCN-NEXT: store
+define amdgpu_kernel void @constant_load_v2half(<2 x half> addrspace(1)* %out, <2 x half> addrspace(2)* %in) #0 {
+  %ld = load <2 x half>, <2 x half> addrspace(2)* %in
----------------
_v2f16


================
Comment at: test/CodeGen/AMDGPU/widen_extending_scalar_loads.ll:161
+; FUNC-LABEL: {{^}}constant_load_v2i8_volatile:
+; GCN: load volatile
+; GCN-NEXT: store
----------------
Needs to check the type


Repository:
  rL LLVM

https://reviews.llvm.org/D35146