[PATCH] D139874: [AMDGPU] Lower VGPR to physical SGPR COPY to S_MOV_B32 if VGPR contains the compile time constant

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:861
+                .addImm(IMMVal);
+            MI.getOperand(1).setReg(TmpReg);
+          }
----------------
arsenm wrote:
>  First of all, you should be just doing addOperand and forwarding whatever was there originally. No need to muck about with conversions or immediates
> 
> The 64-bit case is not solved by simply truncating the immediate. It's materializing a 64-bit value. I'd think you'd be hard pressed to write a a testcase without using MIR (which probably should be done)
If we're copying to a single sgpr and the value doesn't fit, codegen has already gone wrong. We may as well fall through to the current behaviour (which is to miscompile).

If this code path might be copying to pairs of sgprs (I can't tell from the immediate surroundings) then this needs to be significantly more complicated, and probably pick up testing and so forth, but equally that's also broken at present.

I'd rather emit the mov at ISel instead of here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139874