[PATCH] D136663: Handling ADD|SUB U64 decomposed Pseudos not getting lowered to SDWA form

[Yashwant Singh via Phabricator via llvm-commits]

yassingh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp:873
+  if (!TII->canShrink(MISucc, *MRI) &&
+      !TII->getNamedOperand(MI, AMDGPU::OpName::src1)->isReg())
     return;
----------------
rampitec wrote:
> This can be inline literal and still useful I suppose.
Sorry I don't understand, are you suggesting moving this condition to a string literal?


================
Comment at: llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp:929-932
+    BuildMI(MBB, MISucc, MISucc.getDebugLoc(), TII->get(AMDGPU::V_MOV_B32_e32),
+            src1)
+        .addImm(Src1->getImm());
+
----------------
arsenm wrote:
> This is increasing the instruction size (and most likely the code size). This only makes sense to do if we know the fold into the operand can happen. This should perform those legality checks and make the full transform
Yes. There are 2 possible scenarios, if only the src1 operand is immediate then the 'mov' instruction will be folded. If both operands are immediates then 'MOV' will be part of generated assembly. Function 1 and 2 in the both test-files depict these scenarios.

I can add the legality check for this fold happening but then some of the instructions will be missed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136663