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

[Changpeng Fang via Phabricator via llvm-commits]

cfang marked 2 inline comments as done.
cfang 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 &&
----------------
arsenm wrote:
> cfang wrote:
> > arsenm wrote:
> > > 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
> > These _e32 version was generated in "SI Fold Operands". If you think should should not happen, we may need to fix that first.
> This one is complicated. This still needs verification that the vcc use isn't needed
Do you mean the add AMDGPU::V_ADD_I32_e32 will actually been removed?

>This still needs verification that the vcc use isn't needed

How to do this verification?


================
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
----------------
arsenm wrote:
> This shouldn't include a call
Do you mean in general a LIT test shouldn't include a call, or just this test case? I saw a lot in the existing lit tests.


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

https://reviews.llvm.org/D59829