From: Jay Foad Date: Fri, 5 May 2023 09:51:28 +0000 (+0100) Subject: [MachineVerifier] Verify liveins for live-through segments X-Git-Tag: upstream/17.0.6~7384 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2dad1249d2785b08ad21fb00b54721e125434cd8;p=platform%2Fupstream%2Fllvm.git [MachineVerifier] Verify liveins for live-through segments Differential Revision: https://reviews.llvm.org/D149947 --- diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index d6a323d..556047f 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 ba684e2..c2d6b76 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(); 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 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();