[PATCH] D104874: [AMDGPU] Add S_MOV_B64_IMM_PSEUDO for wide constants

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNCombineTupleInits.cpp:94
+
+    switch (I.getOperand(0).getSubReg()) {
+    default:
----------------
arsenm wrote:
> What happens if there are additional defs? I also don't see a dedicated MIR test for this?
If a Def0 or Def1 not null and we need to assign it function exits. Added mir test.


================
Comment at: llvm/lib/Target/AMDGPU/GCNCombineTupleInits.cpp:156
+    RC = MRI->getRegClass(Reg);
+    if (RC->MC->getSizeInBits() != 64 || !TRI->isSGPRClass(RC))
+      continue;
----------------
arsenm wrote:
> Why not also handle the VALU cases?
I have tried by I see no benefits. We can add it later if we see any, I just didn't. Doing this on VALU will also need an EXEC modification scan between defs. In addition we do not always conver a v_mov_b32 into brev, so I have also seen some regressions. That would need to be addressed first too.


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

https://reviews.llvm.org/D104874