[RISCV] Let LiveIntervals::shrinkToUses compute dead AVLs

[lukel97]

We currently manually removing dead AVL defs if they are ADDIs used to materialize immediates > 31.
shrinkToUses already computes dead instructions though, so this patch uses it to generalize to any type of dead def.

[llvmbot]

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

Author: Luke Lau (lukel97)

Changes

We can simplify removing dead AVL immediates > 31 by using the dead argument to shrinkToUses, since it will already compute dead values.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+3-11)

diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 216dc78085204e..f4ff17f7b65585 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -1648,17 +1648,9 @@ bool RISCVCoalesceVSETVLI::coalesceVSETVLIs(MachineBasicBlock &MBB) {
           if (NextMI->getOperand(1).isReg())
             NextMI->getOperand(1).setReg(RISCV::NoRegister);
 
-          if (OldVLReg && OldVLReg.isVirtual()) {
-            // NextMI no longer uses OldVLReg so shrink its LiveInterval.
-            LIS->shrinkToUses(&LIS->getInterval(OldVLReg));
-
-            MachineInstr *VLOpDef = MRI->getUniqueVRegDef(OldVLReg);
-            if (VLOpDef && TII->isAddImmediate(*VLOpDef, OldVLReg) &&
-                MRI->use_nodbg_empty(OldVLReg)) {
-              VLOpDef->eraseFromParent();
-              LIS->removeInterval(OldVLReg);
-            }
-          }
+          // NextMI no longer uses OldVLReg so shrink its LiveInterval.
+          if (OldVLReg && OldVLReg.isVirtual())
+            LIS->shrinkToUses(&LIS->getInterval(OldVLReg), &ToDelete);
           MI.setDesc(NextMI->getDesc());
         }
         MI.getOperand(2).setImm(NextMI->getOperand(2).getImm());

[preames]

I don't think this change is correct. It looks like ShrinkToUses only considers whether definitions are dead, and expects it's callers to validate that the instructions are safe to actually delete. This code doesn't do the required checks, and thus this seemingly obvious change actually introduces a correctness issue.

[lukel97]

Yeah, looks like RegisterCoalescer is the main user of this which eventually passes the dead instructions to LiveRangeEdit::eliminateDeadDefs, which in turn checks for bundles, inline asm and isSafeToMove