[CodeGen] Untangle RegisterCoalescer from LRE's ScannedRemattable flag [nfc[

[preames]

LiveRangeEdit's rematerialization checking logic is used in two quite different ways. For SplitKit and InlineSpiller, we're analyzing all defs associated with a live interval, doing that analysis up front, and then using the result a bit later. The RegisterCoalescer, we're analysing exactly one ValNo at a time, and using the legality result immediately. LRE had a checkRematerializable which existed basically to adapt the later into the former usage model.

Instead, this change bypasses the ScannedRemat and Remattable structures, and directly queries the underlying routines. This is easy to read, and makes it more clear as to which uses actually need the defered analysis. (A following change may try to unwind that too, but it's not strictly NFC.)

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

Author: Philip Reames (preames)

Changes

LiveRangeEdit's rematerialization checking logic is used in two quite different ways. For SplitKit and InlineSpiller, we're analyzing all defs associated with a live interval, doing that analysis up front, and then using the result a bit later. The RegisterCoalescer, we're analysing exactly one ValNo at a time, and using the legality result immediately. LRE had a checkRematerializable which existed basically to adapt the later into the former usage model.

Instead, this change bypasses the ScannedRemat and Remattable structures, and directly queries the underlying routines. This is easy to read, and makes it more clear as to which uses actually need the defered analysis. (A following change may try to unwind that too, but it's not strictly NFC.)

---

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

3 Files Affected:

• (modified) llvm/include/llvm/CodeGen/LiveRangeEdit.h (+2-6)
• (modified) llvm/lib/CodeGen/LiveRangeEdit.cpp (+2-11)
• (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+7-6)

diff --git a/llvm/include/llvm/CodeGen/LiveRangeEdit.h b/llvm/include/llvm/CodeGen/LiveRangeEdit.h
index 3d5df194c71c9..db1785de255f0 100644
--- a/llvm/include/llvm/CodeGen/LiveRangeEdit.h
+++ b/llvm/include/llvm/CodeGen/LiveRangeEdit.h
@@ -176,14 +176,10 @@ class LiveRangeEdit : private MachineRegisterInfo::Delegate {
   Register create() { return createFrom(getReg()); }
 
   /// anyRematerializable - Return true if any parent values may be
-  /// rematerializable.
-  /// This function must be called before any rematerialization is attempted.
+  /// rematerializable.  This function must be called before
+  /// canRematerializeAt is called..
   bool anyRematerializable();
 
-  /// checkRematerializable - Manually add VNI to the list of rematerializable
-  /// values if DefMI may be rematerializable.
-  bool checkRematerializable(VNInfo *VNI, const MachineInstr *DefMI);
-
   /// Remat - Information needed to rematerialize at a specific location.
   struct Remat {
     const VNInfo *const ParentVNI;  // parent_'s value at the remat location.
diff --git a/llvm/lib/CodeGen/LiveRangeEdit.cpp b/llvm/lib/CodeGen/LiveRangeEdit.cpp
index 5514e4eb6cf3e..33e980a5993d3 100644
--- a/llvm/lib/CodeGen/LiveRangeEdit.cpp
+++ b/llvm/lib/CodeGen/LiveRangeEdit.cpp
@@ -68,16 +68,6 @@ Register LiveRangeEdit::createFrom(Register OldReg) {
   return VReg;
 }
 
-bool LiveRangeEdit::checkRematerializable(VNInfo *VNI,
-                                          const MachineInstr *DefMI) {
-  assert(DefMI && "Missing instruction");
-  ScannedRemattable = true;
-  if (!TII.isTriviallyReMaterializable(*DefMI))
-    return false;
-  Remattable.insert(VNI);
-  return true;
-}
-
 void LiveRangeEdit::scanRemattable() {
   for (VNInfo *VNI : getParent().valnos) {
     if (VNI->isUnused())
@@ -90,7 +80,8 @@ void LiveRangeEdit::scanRemattable() {
     MachineInstr *DefMI = LIS.getInstructionFromIndex(OrigVNI->def);
     if (!DefMI)
       continue;
-    checkRematerializable(OrigVNI, DefMI);
+    if (TII.isTriviallyReMaterializable(*DefMI))
+      Remattable.insert(OrigVNI);
   }
   ScannedRemattable = true;
 }
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index b8486f6560c5f..db00f54daeb62 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1325,9 +1325,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
   if (!TII->isAsCheapAsAMove(*DefMI))
     return false;
 
-  SmallVector<Register, 8> NewRegs;
-  LiveRangeEdit Edit(&SrcInt, NewRegs, *MF, *LIS, nullptr, this);
-  if (!Edit.checkRematerializable(ValNo, DefMI))
+  if (!TII->isTriviallyReMaterializable(*DefMI))
     return false;
 
   if (!definesFullReg(*DefMI, SrcReg))
@@ -1395,15 +1393,18 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
     }
   }
 
-  LiveRangeEdit::Remat RM(ValNo);
-  RM.OrigMI = DefMI;
-  if (!Edit.canRematerializeAt(RM, ValNo, CopyIdx))
+  SmallVector<Register, 8> NewRegs;
+  LiveRangeEdit Edit(&SrcInt, NewRegs, *MF, *LIS, nullptr, this);
+  SlotIndex DefIdx = LIS->getInstructionIndex(*DefMI);
+  if (!Edit.allUsesAvailableAt(DefMI, DefIdx, CopyIdx))
     return false;
 
   DebugLoc DL = CopyMI->getDebugLoc();
   MachineBasicBlock *MBB = CopyMI->getParent();
   MachineBasicBlock::iterator MII =
       std::next(MachineBasicBlock::iterator(CopyMI));
+  LiveRangeEdit::Remat RM(ValNo);
+  RM.OrigMI = DefMI;
   Edit.rematerializeAt(*MBB, MII, DstReg, RM, *TRI, false, SrcIdx, CopyMI);
   MachineInstr &NewMI = *std::prev(MII);
   NewMI.setDebugLoc(DL);

[preames]

Note: Landed with failing CI as failures appeared clearly unrelated. Will fetch and build ToT locally to confirm, and revert if not.

Edit: ninja check-llvm passed.