[PATCH] D59829: AMDGPU: An extension to promote constant offset to the immediate

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:1230-1231
+  if (!BaseLoDef ||  !BaseHiDef ||
+      (BaseLoDef->getOpcode() != AMDGPU::V_ADD_I32_e32 &&
+       BaseLoDef->getOpcode() != AMDGPU::V_ADD_I32_e64)   ||
+      (BaseHiDef->getOpcode() != AMDGPU::V_ADDC_U32_e32 &&
----------------
Where are the _e32 versions coming from? I don't think you should be seeing these at this point.

You would need to verify the carry out is dead here. You should add a testcase where the vcc def of the add is used


================
Comment at: test/CodeGen/AMDGPU/promote-constOffset-to-imm.ll:493
+bb:
+  %tmp = tail call i64 @_Z13get_global_idj(i32 0)
+  %tmp1 = and i64 %tmp, 255
----------------
This shouldn't include a call


================
Comment at: test/CodeGen/AMDGPU/promote-constOffset-to-imm.ll:514-517
+bb15:                                             ; preds = %bb15, %bb13
+  %tmp16 = phi i32 [ 0, %bb13 ], [ %tmp96, %bb15 ]
+  %tmp17 = zext i32 %tmp16 to i64
+  %tmp18 = getelementptr inbounds i64, i64 addrspace(1)* %tmp6, i64 %tmp17
----------------
The test could be a lot simpler


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59829/new/

https://reviews.llvm.org/D59829