[PATCH] D70896: AMDGPU: Avoid folding 2 constant operands into an SALU operation

[David Stuttard via Phabricator via llvm-commits]

dstuttard added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:437
+    const MCOperandInfo &OpInfo = InstDesc.OpInfo[OpNo];
+    const SIRegisterInfo &TRI = TII->getRegisterInfo();
+
----------------
arsenm wrote:
> TRI is already a class member
I couldn't work out if you were objecting to the name "TRI" or if you wanted me to re-use TRI from the class object.
I decided to rename it to SRI since to re-use TRI requires a more pervasive change and an extra cast as well.


================
Comment at: llvm/test/CodeGen/AMDGPU/fold-sgpr-multi-imm.mir:12
+    %1:sreg_32 = S_MOV_B32 80
+    %2:sreg_32 = S_ADD_I32 %0, %1, implicit-def $scc
+...
----------------
arsenm wrote:
> Is this just hiding not handling every possible case in tryConstantFoldOp? Can you add a test with a more exotic instruction we're unlikely to ever try to constant fold here?
> 
> Another case that should always break is a frame index operand
I wasn't sure if the instruction I've chosen is now obscure enough - give me a different suggestion if you can think of something better. (Also added the frameindex case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70896