LiveInterval: Make verify functions return bool (#109672)

This will allow the MachineVerifier to check these properties
instead of just asserting

Author: arsenm

llvm/include/llvm/CodeGen/LiveInterval.h

@@ -662,9 +662,9 @@ namespace llvm {
     ///
     /// Note that this is a no-op when asserts are disabled.
 #ifdef NDEBUG
-    void verify() const {}
+    [[nodiscard]] bool verify() const { return true; }
 #else
-    void verify() const;
+    [[nodiscard]] bool verify() const;
 #endif
 
   protected:
@@ -893,9 +893,11 @@ namespace llvm {
     ///
     /// Note that this is a no-op when asserts are disabled.
 #ifdef NDEBUG
-    void verify(const MachineRegisterInfo *MRI = nullptr) const {}
+    [[nodiscard]] bool verify(const MachineRegisterInfo *MRI = nullptr) const {
+      return true;
+    }
 #else
-    void verify(const MachineRegisterInfo *MRI = nullptr) const;
+    [[nodiscard]] bool verify(const MachineRegisterInfo *MRI = nullptr) const;
 #endif
 
   private:

llvm/lib/CodeGen/LiveInterval.cpp

@@ -630,8 +630,8 @@ void LiveRange::join(LiveRange &Other,
                      const int *LHSValNoAssignments,
                      const int *RHSValNoAssignments,
                      SmallVectorImpl<VNInfo *> &NewVNInfo) {
-  verify();
-  Other.verify();
+  assert(verify());
+  assert(Other.verify());
 
   // Determine if any of our values are mapped.  This is uncommon, so we want
   // to avoid the range scan if not.
@@ -797,7 +797,7 @@ void LiveRange::flushSegmentSet() {
       "segment set can be used only initially before switching to the array");
   segments.append(segmentSet->begin(), segmentSet->end());
   segmentSet = nullptr;
-  verify();
+  assert(verify());
 }
 
 bool LiveRange::isLiveAtIndexes(ArrayRef<SlotIndex> Slots) const {
@@ -1055,43 +1055,65 @@ LLVM_DUMP_METHOD void LiveInterval::dump() const {
 #endif
 
 #ifndef NDEBUG
-void LiveRange::verify() const {
+bool LiveRange::verify() const {
   for (const_iterator I = begin(), E = end(); I != E; ++I) {
-    assert(I->start.isValid());
-    assert(I->end.isValid());
-    assert(I->start < I->end);
-    assert(I->valno != nullptr);
-    assert(I->valno->id < valnos.size());
-    assert(I->valno == valnos[I->valno->id]);
+    if (!I->start.isValid())
+      return false;
+    if (!I->end.isValid())
+      return false;
+    if (I->start >= I->end)
+      return false;
+    if (I->valno == nullptr)
+      return false;
+    if (I->valno->id >= valnos.size())
+      return false;
+    if (I->valno != valnos[I->valno->id])
+      return false;
     if (std::next(I) != E) {
-      assert(I->end <= std::next(I)->start);
-      if (I->end == std::next(I)->start)
-        assert(I->valno != std::next(I)->valno);
+      if (I->end > std::next(I)->start)
+        return false;
+      if (I->end == std::next(I)->start) {
+        if (I->valno == std::next(I)->valno)
+          return false;
+      }
     }
   }
+
+  return true;
 }
 
-void LiveInterval::verify(const MachineRegisterInfo *MRI) const {
-  super::verify();
+bool LiveInterval::verify(const MachineRegisterInfo *MRI) const {
+  if (!super::verify())
+    return false;
 
   // Make sure SubRanges are fine and LaneMasks are disjunct.
   LaneBitmask Mask;
   LaneBitmask MaxMask = MRI != nullptr ? MRI->getMaxLaneMaskForVReg(reg())
                                        : LaneBitmask::getAll();
   for (const SubRange &SR : subranges()) {
     // Subrange lanemask should be disjunct to any previous subrange masks.
-    assert((Mask & SR.LaneMask).none());
+    if ((Mask & SR.LaneMask).any())
+      return false;
+
     Mask |= SR.LaneMask;
 
     // subrange mask should not contained in maximum lane mask for the vreg.
-    assert((Mask & ~MaxMask).none());
+    if ((Mask & ~MaxMask).any())
+      return false;
+
     // empty subranges must be removed.
-    assert(!SR.empty());
+    if (SR.empty())
+      return false;
+
+    if (!SR.verify())
+      return false;
 
-    SR.verify();
     // Main liverange should cover subrange.
-    assert(covers(SR));
+    if (!covers(SR))
+      return false;
   }
+
+  return true;
 }
 #endif
 
@@ -1283,7 +1305,7 @@ void LiveRangeUpdater::flush() {
   // Nothing to merge?
   if (Spills.empty()) {
     LR->segments.erase(WriteI, ReadI);
-    LR->verify();
+    assert(LR->verify());
     return;
   }
 
@@ -1301,7 +1323,7 @@ void LiveRangeUpdater::flush() {
   }
   ReadI = WriteI + Spills.size();
   mergeSpills();
-  LR->verify();
+  assert(LR->verify());
 }
 
 unsigned ConnectedVNInfoEqClasses::Classify(const LiveRange &LR) {

llvm/lib/CodeGen/LiveIntervals.cpp

@@ -1105,7 +1105,7 @@ class LiveIntervals::HMEditor {
     else
       handleMoveUp(LR, Reg, LaneMask);
     LLVM_DEBUG(dbgs() << "        -->\t" << LR << '\n');
-    LR.verify();
+    assert(LR.verify());
   }
 
   /// Update LR to reflect an instruction has been moved downwards from OldIdx

llvm/lib/CodeGen/MachineVerifier.cpp

@@ -2758,6 +2758,15 @@ void MachineVerifier::checkLivenessAtUse(const MachineOperand *MO,
                                          Register VRegOrUnit,
                                          LaneBitmask LaneMask) {
   const MachineInstr *MI = MO->getParent();
+
+  if (!LR.verify()) {
+    report("invalid live range", MO, MONum);
+    report_context_liverange(LR);
+    report_context_vreg_regunit(VRegOrUnit);
+    report_context(UseIdx);
+    return;
+  }
+
   LiveQueryResult LRQ = LR.Query(UseIdx);
   bool HasValue = LRQ.valueIn() || (MI->isPHI() && LRQ.valueOut());
   // Check if we have a segment at the use, note however that we only need one
@@ -2784,6 +2793,15 @@ void MachineVerifier::checkLivenessAtDef(const MachineOperand *MO,
                                          Register VRegOrUnit,
                                          bool SubRangeCheck,
                                          LaneBitmask LaneMask) {
+  if (!LR.verify()) {
+    report("invalid live range", MO, MONum);
+    report_context_liverange(LR);
+    report_context_vreg_regunit(VRegOrUnit);
+    if (LaneMask.any())
+      report_context_lanemask(LaneMask);
+    report_context(DefIdx);
+  }
+
   if (const VNInfo *VNI = LR.getVNInfoAt(DefIdx)) {
     // The LR can correspond to the whole reg and its def slot is not obliged
     // to be the same as the MO' def slot. E.g. when we check here "normal"

llvm/lib/CodeGen/RegisterCoalescer.cpp

@@ -3548,8 +3548,7 @@ void RegisterCoalescer::joinSubRegRanges(LiveRange &LRange, LiveRange &RRange,
   LHSVals.removeImplicitDefs();
   RHSVals.removeImplicitDefs();
 
-  LRange.verify();
-  RRange.verify();
+  assert(LRange.verify() && RRange.verify());
 
   // Join RRange into LHS.
   LRange.join(RRange, LHSVals.getAssignments(), RHSVals.getAssignments(),

llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp

@@ -397,7 +397,7 @@ void GCNRewritePartialRegUses::updateLiveIntervals(Register OldReg,
   }
   if (NewLI.empty())
     NewLI.assign(OldLI, Allocator);
-  NewLI.verify(MRI);
+  assert(NewLI.verify(MRI));
   LIS->removeInterval(OldReg);
 }
 

llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp

@@ -568,7 +568,7 @@ void HexagonExpandCondsets::updateLiveness(const std::set<Register> &RegSet,
     // after that.
     if (UpdateKills)
       updateKillFlags(R);
-    LIS->getInterval(R).verify();
+    assert(LIS->getInterval(R).verify());
   }
 }
 
@@ -1197,7 +1197,7 @@ bool HexagonExpandCondsets::coalesceRegisters(RegisterRef R1, RegisterRef R2) {
 
   updateKillFlags(R1.Reg);
   LLVM_DEBUG(dbgs() << "coalesced: " << L1 << "\n");
-  L1.verify();
+  assert(L1.verify());
 
   return true;
 }