[RISCV][VLOpt] Minor worklist invariant cleanup [NFC]

[preames]

In retrospect, this probably should have been rolled into #123973. It seemed more involved when I first decided to split. :)

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Philip Reames (preames)

Changes

In retrospect, this probably should have been rolled into #123973. It seemed more involved when I first decided to split. :)

---

Full diff: https://github.com/llvm/llvm-project/pull/123989.diff

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp (+7-8)

diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index 66d26bf5b11e2d..529bffcae1319e 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -1189,6 +1189,10 @@ bool RISCVVLOptimizer::isCandidate(const MachineInstr &MI) const {
     return false;
   }
 
+  assert(MI.getOperand(0).isReg() &&
+         isVectorRegClass(MI.getOperand(0).getReg(), MRI) &&
+         "All supported instructions produce a vector register result");
+
   LLVM_DEBUG(dbgs() << "Found a candidate for VL reduction: " << MI << "\n");
   return true;
 }
@@ -1295,9 +1299,6 @@ std::optional<MachineOperand> RISCVVLOptimizer::checkUsers(MachineInstr &MI) {
 bool RISCVVLOptimizer::tryReduceVL(MachineInstr &MI) {
   LLVM_DEBUG(dbgs() << "Trying to reduce VL for " << MI << "\n");
 
-  if (!isVectorRegClass(MI.getOperand(0).getReg(), MRI))
-    return false;
-
   auto CommonVL = checkUsers(MI);
   if (!CommonVL)
     return false;
@@ -1347,14 +1348,11 @@ bool RISCVVLOptimizer::runOnMachineFunction(MachineFunction &MF) {
   auto PushOperands = [this, &Worklist](MachineInstr &MI,
                                         bool IgnoreSameBlock) {
     for (auto &Op : MI.operands()) {
-      if (!Op.isReg() || !Op.isUse() || !Op.getReg().isVirtual())
-        continue;
-
-      if (!isVectorRegClass(Op.getReg(), MRI))
+      if (!Op.isReg() || !Op.isUse() || !Op.getReg().isVirtual() ||
+          !isVectorRegClass(Op.getReg(), MRI))
         continue;
 
       MachineInstr *DefMI = MRI->getVRegDef(Op.getReg());
-
       if (!isCandidate(*DefMI))
         continue;
 
@@ -1388,6 +1386,7 @@ bool RISCVVLOptimizer::runOnMachineFunction(MachineFunction &MF) {
   while (!Worklist.empty()) {
     assert(MadeChange);
     MachineInstr &MI = *Worklist.pop_back_val();
+    assert(isCandidate(MI));
     if (!tryReduceVL(MI))
       continue;
     PushOperands(MI, /*IgnoreSameBlock*/ false);

[michaelmaitland]

LGTM with minor point.

[michaelmaitland]

Is this assert redundant? isCandidate explicitly blocks insertion to the Worklist in PushOperands. That being said, isCandidate is cheap, so I don't have a strong objection.

[preames]

I can drop it if you want. Personally, I find it helpful to have the assertion at the point of extraction from a worklist. This is particular true when there's more than one insert point though - which doesn't apply here. Happy to defer to your preference.

[michaelmaitland]

We can keep it. It is cheap.