[llvm-branch-commits] [llvm] f264f9a - [SlotIndexes] Fix and simplify basic block splitting

Jay Foad via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Jan 12 03:06:27 PST 2021


Author: Jay Foad
Date: 2021-01-12T10:50:14Z
New Revision: f264f9ad7df538357dfc8c5f318c5c8b0df3d99f

URL: https://github.com/llvm/llvm-project/commit/f264f9ad7df538357dfc8c5f318c5c8b0df3d99f
DIFF: https://github.com/llvm/llvm-project/commit/f264f9ad7df538357dfc8c5f318c5c8b0df3d99f.diff

LOG: [SlotIndexes] Fix and simplify basic block splitting

Remove the InsertionPoint argument from SlotIndexes::insertMBBInMaps
because it was confusing: what does it mean to insert a new block
between two instructions, in the middle of an existing block?

Instead, support the case that MachineBasicBlock::splitAt really needs,
where the new block contains some instructions that are already in the
maps because they have been moved there from the tail of the previous
block.

In all other use cases the new block is empty.

Based on work by Carl Ritson!

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

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/LiveIntervals.h
    llvm/include/llvm/CodeGen/SlotIndexes.h
    llvm/lib/CodeGen/MachineBasicBlock.cpp
    llvm/unittests/MI/LiveIntervalTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/LiveIntervals.h b/llvm/include/llvm/CodeGen/LiveIntervals.h
index 1a6b59a8959e..fa08166791b0 100644
--- a/llvm/include/llvm/CodeGen/LiveIntervals.h
+++ b/llvm/include/llvm/CodeGen/LiveIntervals.h
@@ -256,9 +256,8 @@ class VirtRegMap;
       return Indexes->getMBBFromIndex(index);
     }
 
-    void insertMBBInMaps(MachineBasicBlock *MBB,
-                         MachineInstr *InsertionPoint = nullptr) {
-      Indexes->insertMBBInMaps(MBB, InsertionPoint);
+    void insertMBBInMaps(MachineBasicBlock *MBB) {
+      Indexes->insertMBBInMaps(MBB);
       assert(unsigned(MBB->getNumber()) == RegMaskBlocks.size() &&
              "Blocks must be added in order.");
       RegMaskBlocks.push_back(std::make_pair(RegMaskSlots.size(), 0));

diff  --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 19eab7ae5e35..b2133de93ea2 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -604,38 +604,27 @@ class raw_ostream;
     }
 
     /// Add the given MachineBasicBlock into the maps.
-    /// If \p InsertionPoint is specified then the block will be placed
-    /// before the given machine instr, otherwise it will be placed
-    /// before the next block in MachineFunction insertion order.
-    void insertMBBInMaps(MachineBasicBlock *mbb,
-                         MachineInstr *InsertionPoint = nullptr) {
-      MachineFunction::iterator nextMBB =
-        std::next(MachineFunction::iterator(mbb));
-
-      IndexListEntry *startEntry = nullptr;
-      IndexListEntry *endEntry = nullptr;
-      IndexList::iterator newItr;
-      if (InsertionPoint) {
-        startEntry = createEntry(nullptr, 0);
-        endEntry = getInstructionIndex(*InsertionPoint).listEntry();
-        newItr = indexList.insert(endEntry->getIterator(), startEntry);
-      } else if (nextMBB == mbb->getParent()->end()) {
-        startEntry = &indexList.back();
-        endEntry = createEntry(nullptr, 0);
-        newItr = indexList.insertAfter(startEntry->getIterator(), endEntry);
-      } else {
-        startEntry = createEntry(nullptr, 0);
-        endEntry = getMBBStartIdx(&*nextMBB).listEntry();
-        newItr = indexList.insert(endEntry->getIterator(), startEntry);
-      }
+    /// If it contains any instructions then they must already be in the maps.
+    /// This is used after a block has been split by moving some suffix of its
+    /// instructions into a newly created block.
+    void insertMBBInMaps(MachineBasicBlock *mbb) {
+      assert(mbb != &mbb->getParent()->front() &&
+             "Can't insert a new block at the beginning of a function.");
+      auto prevMBB = std::prev(MachineFunction::iterator(mbb));
+
+      // Create a new entry to be used for the start of mbb and the end of
+      // prevMBB.
+      IndexListEntry *startEntry = createEntry(nullptr, 0);
+      IndexListEntry *endEntry = getMBBEndIdx(&*prevMBB).listEntry();
+      IndexListEntry *insEntry =
+          mbb->empty() ? endEntry
+                       : getInstructionIndex(mbb->front()).listEntry();
+      IndexList::iterator newItr =
+          indexList.insert(insEntry->getIterator(), startEntry);
 
       SlotIndex startIdx(startEntry, SlotIndex::Slot_Block);
       SlotIndex endIdx(endEntry, SlotIndex::Slot_Block);
 
-      MachineFunction::iterator prevMBB(mbb);
-      assert(prevMBB != mbb->getParent()->end() &&
-             "Can't insert a new block at the beginning of a function.");
-      --prevMBB;
       MBBRanges[prevMBB->getNumber()].second = startIdx;
 
       assert(unsigned(mbb->getNumber()) == MBBRanges.size() &&

diff  --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index c7b404e075e1..fded4b15e67b 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -980,7 +980,7 @@ MachineBasicBlock *MachineBasicBlock::splitAt(MachineInstr &MI,
     addLiveIns(*SplitBB, LiveRegs);
 
   if (LIS)
-    LIS->insertMBBInMaps(SplitBB, &MI);
+    LIS->insertMBBInMaps(SplitBB);
 
   return SplitBB;
 }

diff  --git a/llvm/unittests/MI/LiveIntervalTest.cpp b/llvm/unittests/MI/LiveIntervalTest.cpp
index 3971d86e82d3..d367ee4676a1 100644
--- a/llvm/unittests/MI/LiveIntervalTest.cpp
+++ b/llvm/unittests/MI/LiveIntervalTest.cpp
@@ -149,6 +149,19 @@ static void testHandleMoveIntoNewBundle(MachineFunction &MF, LiveIntervals &LIS,
   LIS.handleMoveIntoNewBundle(*BundleStart, true);
 }
 
+/**
+ * Split block numbered \p BlockNum at instruction \p SplitAt using
+ * MachineBasicBlock::splitAt updating liveness intervals.
+ */
+static void testSplitAt(MachineFunction &MF, LiveIntervals &LIS,
+                        unsigned SplitAt, unsigned BlockNum) {
+  MachineInstr &SplitInstr = getMI(MF, SplitAt, BlockNum);
+  MachineBasicBlock &MBB = *SplitInstr.getParent();
+
+  // Split block and update live intervals
+  MBB.splitAt(SplitInstr, false, &LIS);
+}
+
 static void liveIntervalTest(StringRef MIRFunc, LiveIntervalTest T) {
   LLVMContext Context;
   std::unique_ptr<LLVMTargetMachine> TM = createTargetMachine();
@@ -608,6 +621,34 @@ TEST(LiveIntervalTest, BundleSubRegDef) {
   });
 }
 
+TEST(LiveIntervalTest, SplitAtOneInstruction) {
+  liveIntervalTest(R"MIR(
+    successors: %bb.1
+    %0 = IMPLICIT_DEF
+    S_BRANCH %bb.1
+  bb.1:
+    S_NOP 0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+    testSplitAt(MF, LIS, 1, 0);
+  });
+}
+
+TEST(LiveIntervalTest, SplitAtMultiInstruction) {
+  liveIntervalTest(R"MIR(
+    successors: %bb.1
+    %0 = IMPLICIT_DEF
+    S_NOP 0
+    S_NOP 0
+    S_NOP 0
+    S_NOP 0
+    S_BRANCH %bb.1
+  bb.1:
+    S_NOP 0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+    testSplitAt(MF, LIS, 0, 0);
+  });
+}
+
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
   initLLVM();


        


More information about the llvm-branch-commits mailing list