[MachineVerifier] Verify liveins for live-through segments

Differential Revision: https://reviews.llvm.org/D149947

2 files changed, 126 insertions, 89 deletions

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<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();