LiveInterval: Make verify functions return bool #109672
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will allow the MachineVerifier to check these properties instead of just asserting
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-backend-hexagon @llvm/pr-subscribers-llvm-regalloc Author: Matt Arsenault (arsenm) ChangesThis will allow the MachineVerifier to check these properties Full diff: https://github.com/llvm/llvm-project/pull/109672.diff 7 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/LiveInterval.h b/llvm/include/llvm/CodeGen/LiveInterval.h
index ad8dec68c073c0..cd75de4e2df852 100644
--- a/llvm/include/llvm/CodeGen/LiveInterval.h
+++ b/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:
diff --git a/llvm/lib/CodeGen/LiveInterval.cpp b/llvm/lib/CodeGen/LiveInterval.cpp
index c81540602f59b3..0683353d9cdbab 100644
--- a/llvm/lib/CodeGen/LiveInterval.cpp
+++ b/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,24 +1055,36 @@ 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;
@@ -1080,18 +1092,28 @@ void LiveInterval::verify(const MachineRegisterInfo *MRI) const {
: 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) {
diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp
index d879a7c1fb37e9..7ddaaaa915ef17 100644
--- a/llvm/lib/CodeGen/LiveIntervals.cpp
+++ b/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
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 27664207d1e696..e1295ec8ea6e9a 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/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"
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 97f8346df0e8fe..99125200c1a4f1 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/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(),
diff --git a/llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp b/llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp
index 90169b1cb3df9b..66c05b43340848 100644
--- a/llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp
+++ b/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);
}
diff --git a/llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp b/llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp
index 8cf853ad0110d1..06f6abe5bf8476 100644
--- a/llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp
+++ b/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;
}
|
qcolombet
approved these changes
Sep 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will allow the MachineVerifier to check these properties
instead of just asserting