[PATCH] D119696: [AMDGPU] Improve v_cmpx usage on GFX10.3.

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added a comment.

Can you add lit tests for the fixes you made please?



================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:325
+
+// Determine if an register is in use from the end of an basic block
+// until an specific instruction occurs.
----------------
Typo: an register, an basic block, an specific instruction

I was a bit confused by the comment, maybe calling the function `isRegisterLiveAfter(MachineInstr &Stop, MCRegister Reg, …)` is more clear? (The MBB can be fetched from Stop.getParent())

I think this function checks if Reg is still live/used after Stop?


================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:333-335
+  MachineBasicBlock::reverse_iterator A = MBB.rend();
+
+  for (++A; A != Stop; ++A) {
----------------
I think a reverse iterator iterates from MBB.rbegin() to MBB.rend() (I’m always confused by this, but it’s used in SIOptimizeExecMaskingPreRA.cpp:412 in this way).


================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:379
+  if (Src0->isReg() && TRI->isSGPRReg(MRI, Src0->getReg()) &&
+      SaveExec.modifiesRegister(Src0->getReg()))
+    return nullptr;
----------------
Does this also need the TRI?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119696