[PATCH] D54882: [AMDGPU] Add sdwa support for ADD|SUB U64 decomposed Pseudos

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:862
+  // if no reference to VCC in current Func, its usable.
+  if (MRI->reg_empty(AMDGPU::VCC))
+    return true;
----------------
Not necessarily, a vcc_lo or hcc_hi can be used.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:868
+  bool Cand1 = false;
+  for (MachineBasicBlock::const_instr_iterator I = MBB->instr_begin(),
+                                               E = MBB->instr_end();
----------------
There is absolutely no guarantee vcc is not live at the beginning of the block.
You need to query liveness before MI (MBB::computeRegisterLiveness) and only scan from MI to MISucc.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:879
+      if (Instr.readsRegister(AMDGPU::VCC, TRI))
+        VCCInUse = false;
+    }
----------------
Use does not kill register, it is not a destructive read.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:942
+  // Give up if any mods on MI or MISucc.
+  if (auto *Mod = TII->getNamedOperand(MI, AMDGPU::OpName::src0_modifiers))
+    return;
----------------
You do not need these checks. First the presence of modifiers does not prevent the shrink. When they are non-zero, that prevents it. And this is already tested by canShrink().


================
Comment at: test/CodeGen/AMDGPU/sdwa-ops.mir:1
+# RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass=si-peephole-sdwa -o - %s | FileCheck -check-prefix=GFX9 %s
+# RUN: llc -march=amdgcn -mcpu=fiji -run-pass=si-peephole-sdwa -o - %s | FileCheck -check-prefix=GFX9 %s
----------------
I do not see a test with modifiers.


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

https://reviews.llvm.org/D54882