[llvm] 2dad124 - [MachineVerifier] Verify liveins for live-through segments
Jay Foad via llvm-commits
llvm-commits at lists.llvm.org
Wed May 24 07:18:27 PDT 2023
Author: Jay Foad
Date: 2023-05-24T15:17:02+01:00
New Revision: 2dad1249d2785b08ad21fb00b54721e125434cd8
URL: https://github.com/llvm/llvm-project/commit/2dad1249d2785b08ad21fb00b54721e125434cd8
DIFF: https://github.com/llvm/llvm-project/commit/2dad1249d2785b08ad21fb00b54721e125434cd8.diff
LOG: [MachineVerifier] Verify liveins for live-through segments
Differential Revision: https://reviews.llvm.org/D149947
Added:
Modified:
llvm/lib/CodeGen/MachineVerifier.cpp
llvm/unittests/MI/LiveIntervalTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index d6a323db250e6..556047feca73d 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -3107,108 +3107,109 @@ void MachineVerifier::verifyLiveRangeSegment(const LiveRange &LR,
return;
}
- // No more checks for live-out segments.
- if (S.end == LiveInts->getMBBEndIdx(EndMBB))
- return;
-
- // RegUnit intervals are allowed dead phis.
- if (!Reg.isVirtual() && VNI->isPHIDef() && S.start == VNI->def &&
- S.end == VNI->def.getDeadSlot())
- return;
-
- // The live segment is ending inside EndMBB
- const MachineInstr *MI =
- LiveInts->getInstructionFromIndex(S.end.getPrevSlot());
- if (!MI) {
- report("Live segment doesn't end at a valid instruction", EndMBB);
- report_context(LR, Reg, LaneMask);
- report_context(S);
- return;
- }
-
- // The block slot must refer to a basic block boundary.
- if (S.end.isBlock()) {
- report("Live segment ends at B slot of an instruction", EndMBB);
- report_context(LR, Reg, LaneMask);
- report_context(S);
- }
+ // Checks for non-live-out segments.
+ if (S.end != LiveInts->getMBBEndIdx(EndMBB)) {
+ // RegUnit intervals are allowed dead phis.
+ if (!Reg.isVirtual() && VNI->isPHIDef() && S.start == VNI->def &&
+ S.end == VNI->def.getDeadSlot())
+ return;
- if (S.end.isDead()) {
- // Segment ends on the dead slot.
- // That means there must be a dead def.
- if (!SlotIndex::isSameInstr(S.start, S.end)) {
- report("Live segment ending at dead slot spans instructions", EndMBB);
+ // The live segment is ending inside EndMBB
+ const MachineInstr *MI =
+ LiveInts->getInstructionFromIndex(S.end.getPrevSlot());
+ if (!MI) {
+ report("Live segment doesn't end at a valid instruction", EndMBB);
report_context(LR, Reg, LaneMask);
report_context(S);
+ return;
}
- }
- // After tied operands are rewritten, a live segment can only end at an
- // early-clobber slot if it is being redefined by an early-clobber def.
- // TODO: Before tied operands are rewritten, a live segment can only end at an
- // early-clobber slot if the last use is tied to an early-clobber def.
- if (MF->getProperties().hasProperty(
- MachineFunctionProperties::Property::TiedOpsRewritten) &&
- S.end.isEarlyClobber()) {
- if (I+1 == LR.end() || (I+1)->start != S.end) {
- report("Live segment ending at early clobber slot must be "
- "redefined by an EC def in the same instruction", EndMBB);
+ // The block slot must refer to a basic block boundary.
+ if (S.end.isBlock()) {
+ report("Live segment ends at B slot of an instruction", EndMBB);
report_context(LR, Reg, LaneMask);
report_context(S);
}
- }
- // The following checks only apply to virtual registers. Physreg liveness
- // is too weird to check.
- if (Reg.isVirtual()) {
- // A live segment can end with either a redefinition, a kill flag on a
- // use, or a dead flag on a def.
- bool hasRead = false;
- bool hasSubRegDef = false;
- bool hasDeadDef = false;
- for (ConstMIBundleOperands MOI(*MI); MOI.isValid(); ++MOI) {
- if (!MOI->isReg() || MOI->getReg() != Reg)
- continue;
- unsigned Sub = MOI->getSubReg();
- LaneBitmask SLM = Sub != 0 ? TRI->getSubRegIndexLaneMask(Sub)
- : LaneBitmask::getAll();
- if (MOI->isDef()) {
- if (Sub != 0) {
- hasSubRegDef = true;
- // An operand %0:sub0 reads %0:sub1..n. Invert the lane
- // mask for subregister defs. Read-undef defs will be handled by
- // readsReg below.
- SLM = ~SLM;
- }
- if (MOI->isDead())
- hasDeadDef = true;
+ if (S.end.isDead()) {
+ // Segment ends on the dead slot.
+ // That means there must be a dead def.
+ if (!SlotIndex::isSameInstr(S.start, S.end)) {
+ report("Live segment ending at dead slot spans instructions", EndMBB);
+ report_context(LR, Reg, LaneMask);
+ report_context(S);
}
- if (LaneMask.any() && (LaneMask & SLM).none())
- continue;
- if (MOI->readsReg())
- hasRead = true;
}
- if (S.end.isDead()) {
- // Make sure that the corresponding machine operand for a "dead" live
- // range has the dead flag. We cannot perform this check for subregister
- // liveranges as partially dead values are allowed.
- if (LaneMask.none() && !hasDeadDef) {
- report("Instruction ending live segment on dead slot has no dead flag",
- MI);
+
+ // After tied operands are rewritten, a live segment can only end at an
+ // early-clobber slot if it is being redefined by an early-clobber def.
+ // TODO: Before tied operands are rewritten, a live segment can only end at
+ // an early-clobber slot if the last use is tied to an early-clobber def.
+ if (MF->getProperties().hasProperty(
+ MachineFunctionProperties::Property::TiedOpsRewritten) &&
+ S.end.isEarlyClobber()) {
+ if (I + 1 == LR.end() || (I + 1)->start != S.end) {
+ report("Live segment ending at early clobber slot must be "
+ "redefined by an EC def in the same instruction",
+ EndMBB);
report_context(LR, Reg, LaneMask);
report_context(S);
}
- } else {
- if (!hasRead) {
- // When tracking subregister liveness, the main range must start new
- // values on partial register writes, even if there is no read.
- if (!MRI->shouldTrackSubRegLiveness(Reg) || LaneMask.any() ||
- !hasSubRegDef) {
- report("Instruction ending live segment doesn't read the register",
- MI);
+ }
+
+ // The following checks only apply to virtual registers. Physreg liveness
+ // is too weird to check.
+ if (Reg.isVirtual()) {
+ // A live segment can end with either a redefinition, a kill flag on a
+ // use, or a dead flag on a def.
+ bool hasRead = false;
+ bool hasSubRegDef = false;
+ bool hasDeadDef = false;
+ for (ConstMIBundleOperands MOI(*MI); MOI.isValid(); ++MOI) {
+ if (!MOI->isReg() || MOI->getReg() != Reg)
+ continue;
+ unsigned Sub = MOI->getSubReg();
+ LaneBitmask SLM =
+ Sub != 0 ? TRI->getSubRegIndexLaneMask(Sub) : LaneBitmask::getAll();
+ if (MOI->isDef()) {
+ if (Sub != 0) {
+ hasSubRegDef = true;
+ // An operand %0:sub0 reads %0:sub1..n. Invert the lane
+ // mask for subregister defs. Read-undef defs will be handled by
+ // readsReg below.
+ SLM = ~SLM;
+ }
+ if (MOI->isDead())
+ hasDeadDef = true;
+ }
+ if (LaneMask.any() && (LaneMask & SLM).none())
+ continue;
+ if (MOI->readsReg())
+ hasRead = true;
+ }
+ if (S.end.isDead()) {
+ // Make sure that the corresponding machine operand for a "dead" live
+ // range has the dead flag. We cannot perform this check for subregister
+ // liveranges as partially dead values are allowed.
+ if (LaneMask.none() && !hasDeadDef) {
+ report(
+ "Instruction ending live segment on dead slot has no dead flag",
+ MI);
report_context(LR, Reg, LaneMask);
report_context(S);
}
+ } else {
+ if (!hasRead) {
+ // When tracking subregister liveness, the main range must start new
+ // values on partial register writes, even if there is no read.
+ if (!MRI->shouldTrackSubRegLiveness(Reg) || LaneMask.any() ||
+ !hasSubRegDef) {
+ report("Instruction ending live segment doesn't read the register",
+ MI);
+ report_context(LR, Reg, LaneMask);
+ report_context(S);
+ }
+ }
}
}
}
diff --git a/llvm/unittests/MI/LiveIntervalTest.cpp b/llvm/unittests/MI/LiveIntervalTest.cpp
index ba684e2b8f4ef..c2d6b7660c892 100644
--- a/llvm/unittests/MI/LiveIntervalTest.cpp
+++ b/llvm/unittests/MI/LiveIntervalTest.cpp
@@ -83,14 +83,16 @@ struct TestPass : public MachineFunctionPass {
// We should never call this but always use PM.add(new TestPass(...))
abort();
}
- TestPass(LiveIntervalTest T) : MachineFunctionPass(ID), T(T) {
+ TestPass(LiveIntervalTest T, bool ShouldPass)
+ : MachineFunctionPass(ID), T(T), ShouldPass(ShouldPass) {
initializeTestPassPass(*PassRegistry::getPassRegistry());
}
bool runOnMachineFunction(MachineFunction &MF) override {
LiveIntervals &LIS = getAnalysis<LiveIntervals>();
T(MF, LIS);
- EXPECT_TRUE(MF.verify(this));
+ EXPECT_EQ(MF.verify(this, /* Banner */ nullptr, /* AbortOnError */ false),
+ ShouldPass);
return true;
}
@@ -102,6 +104,7 @@ struct TestPass : public MachineFunctionPass {
}
private:
LiveIntervalTest T;
+ bool ShouldPass;
};
static MachineInstr &getMI(MachineFunction &MF, unsigned At,
@@ -185,7 +188,8 @@ static bool checkRegUnitInterference(LiveIntervals &LIS,
return false;
}
-static void liveIntervalTest(StringRef MIRFunc, LiveIntervalTest T) {
+static void liveIntervalTest(StringRef MIRFunc, LiveIntervalTest T,
+ bool ShouldPass = true) {
LLVMContext Context;
std::unique_ptr<LLVMTargetMachine> TM = createTargetMachine();
// This test is designed for the X86 backend; stop if it is not available.
@@ -209,7 +213,7 @@ body: |
"func");
ASSERT_TRUE(M);
- PM.add(new TestPass(T));
+ PM.add(new TestPass(T, ShouldPass));
PM.run(*M);
}
@@ -742,6 +746,38 @@ TEST(LiveIntervalTest, AdjacentIntervals) {
});
}
+TEST(LiveIntervalTest, LiveThroughSegments) {
+ liveIntervalTest(
+ R"MIR(
+ %0 = IMPLICIT_DEF
+ S_BRANCH %bb.2
+ bb.1:
+ S_NOP 0, implicit %0
+ S_ENDPGM 0
+ bb.2:
+ S_BRANCH %bb.1
+)MIR",
+ [](MachineFunction &MF, LiveIntervals &LIS) {
+ MachineInstr &ImpDef = getMI(MF, 0, 0);
+ MachineInstr &Nop = getMI(MF, 0, 1);
+ LiveInterval &LI = LIS.getInterval(ImpDef.getOperand(0).getReg());
+ SlotIndex OrigIdx = LIS.getInstructionIndex(ImpDef).getRegSlot();
+ LiveInterval::iterator FirstSeg = LI.FindSegmentContaining(OrigIdx);
+
+ // %0 is live through bb.2. Move its def into bb.1 and update LIS but do
+ // not remove the segment for bb.2. This should cause machine
+ // verification to fail.
+ LIS.RemoveMachineInstrFromMaps(ImpDef);
+ ImpDef.moveBefore(&Nop);
+ LIS.InsertMachineInstrInMaps(ImpDef);
+
+ SlotIndex NewIdx = LIS.getInstructionIndex(ImpDef).getRegSlot();
+ FirstSeg->start = NewIdx;
+ FirstSeg->valno->def = NewIdx;
+ },
+ false);
+}
+
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
initLLVM();
More information about the llvm-commits
mailing list