[PATCH] D33583: [AMDGPU] Allow SDWA in instructions with immediates and SGPRs

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:590
 
-  // 2. Are all operands - VGPRs
-  for (const MachineOperand &Operand : MI.explicit_operands()) {
-    if (!Operand.isReg() || !TRI->isVGPR(*MRI, Operand.getReg()))
+  // 2. Are all operands - VGPRs or can be changed to VGPRs
+  const MCInstrDesc &Desc = TII->get(MI.getOpcode());
----------------
SamWot wrote:
> I think there should be some heuristic to check if this should be done. E.g. if we would fold only one SDWA operand in this instruction then creating this copy would only increase code size.
Copy can be hoisted out of a loop. In this situation it is still profitable even if code has grown. I.e. there is a chance to improve, but if not it does not really hurt.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:596-597
+        (Operand.isReg() && !TRI->isVGPR(*MRI, Operand.getReg()))) {
+      if (Desc.OpInfo[I].RegClass == -1 ||
+         !TRI->hasVGPRs(TRI->getRegClass(Desc.OpInfo[I].RegClass)))
+        return false;
----------------
SamWot wrote:
> Why this check is needed? It seems redundant for me.
Is there any guarantee that all operands can be VGPRs?


Repository:
  rL LLVM

https://reviews.llvm.org/D33583