[llvm] 43e2460 - [LiveIntervals] Replace handleMoveIntoBundle
Carl Ritson via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 16 03:59:49 PDT 2020
Author: Carl Ritson
Date: 2020-04-16T19:58:19+09:00
New Revision: 43e2460a89abf6aace35973c682e1723d5f16f10
URL: https://github.com/llvm/llvm-project/commit/43e2460a89abf6aace35973c682e1723d5f16f10
DIFF: https://github.com/llvm/llvm-project/commit/43e2460a89abf6aace35973c682e1723d5f16f10.diff
LOG: [LiveIntervals] Replace handleMoveIntoBundle
Summary:
The current handleMoveIntoBundle implementation is unusable,
it attempts to access the slot indexes of bundled instructions.
It also leaves bundled instructions with slot indexes assigned.
Replace handleMoveIntoBundle this with a more explicit
handleMoveIntoNewBundle function which recalculates the live
intervals for all instructions moved into a newly formed bundle,
and removes slot indexes from these instructions.
Reviewers: arsenm, MaskRay, kariddi, tpr, qcolombet
Reviewed By: qcolombet
Subscribers: MatzeB, wdng, hiraditya, arphaman, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77969
Added:
Modified:
llvm/include/llvm/CodeGen/LiveIntervals.h
llvm/include/llvm/CodeGen/SlotIndexes.h
llvm/lib/CodeGen/LiveIntervals.cpp
llvm/lib/CodeGen/SlotIndexes.cpp
llvm/unittests/MI/LiveIntervalTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/LiveIntervals.h b/llvm/include/llvm/CodeGen/LiveIntervals.h
index 34e88045f6ef..1d89e6f38c72 100644
--- a/llvm/include/llvm/CodeGen/LiveIntervals.h
+++ b/llvm/include/llvm/CodeGen/LiveIntervals.h
@@ -310,16 +310,16 @@ class VirtRegMap;
/// \param UpdateFlags Update live intervals for nonallocatable physregs.
void handleMove(MachineInstr &MI, bool UpdateFlags = false);
- /// Update intervals for operands of \p MI so that they begin/end on the
- /// SlotIndex for \p BundleStart.
+ /// Update intervals of operands of all instructions in the newly
+ /// created bundle specified by \p BundleStart.
///
/// \param UpdateFlags Update live intervals for nonallocatable physregs.
///
- /// Requires MI and BundleStart to have SlotIndexes, and assumes
- /// existing liveness is accurate. BundleStart should be the first
- /// instruction in the Bundle.
- void handleMoveIntoBundle(MachineInstr &MI, MachineInstr &BundleStart,
- bool UpdateFlags = false);
+ /// Assumes existing liveness is accurate.
+ /// \pre BundleStart should be the first instruction in the Bundle.
+ /// \pre BundleStart should not have a have SlotIndex as one will be assigned.
+ void handleMoveIntoNewBundle(MachineInstr &BundleStart,
+ bool UpdateFlags = false);
/// Update live intervals for instructions in a range of iterators. It is
/// intended for use after target hooks that may insert or remove
diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index fb833806ca8e..85bd7a404f9b 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -382,13 +382,15 @@ class raw_ostream;
}
/// Returns the base index for the given instruction.
- SlotIndex getInstructionIndex(const MachineInstr &MI) const {
+ SlotIndex getInstructionIndex(const MachineInstr &MI,
+ bool IgnoreBundle = false) const {
// Instructions inside a bundle have the same number as the bundle itself.
auto BundleStart = getBundleStart(MI.getIterator());
auto BundleEnd = getBundleEnd(MI.getIterator());
// Use the first non-debug instruction in the bundle to get SlotIndex.
const MachineInstr &BundleNonDebug =
- *skipDebugInstructionsForward(BundleStart, BundleEnd);
+ IgnoreBundle ? MI
+ : *skipDebugInstructionsForward(BundleStart, BundleEnd);
assert(!BundleNonDebug.isDebugInstr() &&
"Could not use a debug instruction to query mi2iMap.");
Mi2IndexMap::const_iterator itr = mi2iMap.find(&BundleNonDebug);
@@ -573,7 +575,11 @@ class raw_ostream;
/// Removes machine instruction (bundle) \p MI from the mapping.
/// This should be called before MachineInstr::eraseFromParent() is used to
/// remove a whole bundle or an unbundled instruction.
- void removeMachineInstrFromMaps(MachineInstr &MI);
+ /// If \p AllowBundled is set then this can be used on a bundled
+ /// instruction; however, this exists to support handleMoveIntoBundle,
+ /// and in general removeSingleMachineInstrFromMaps should be used instead.
+ void removeMachineInstrFromMaps(MachineInstr &MI,
+ bool AllowBundled = false);
/// Removes a single machine instruction \p MI from the mapping.
/// This should be called before MachineInstr::eraseFromBundle() is used to
diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp
index 70f131e406ef..b830c93d43f4 100644
--- a/llvm/lib/CodeGen/LiveIntervals.cpp
+++ b/llvm/lib/CodeGen/LiveIntervals.cpp
@@ -1478,13 +1478,43 @@ void LiveIntervals::handleMove(MachineInstr &MI, bool UpdateFlags) {
HME.updateAllRanges(&MI);
}
-void LiveIntervals::handleMoveIntoBundle(MachineInstr &MI,
- MachineInstr &BundleStart,
- bool UpdateFlags) {
- SlotIndex OldIndex = Indexes->getInstructionIndex(MI);
- SlotIndex NewIndex = Indexes->getInstructionIndex(BundleStart);
- HMEditor HME(*this, *MRI, *TRI, OldIndex, NewIndex, UpdateFlags);
- HME.updateAllRanges(&MI);
+void LiveIntervals::handleMoveIntoNewBundle(MachineInstr &BundleStart,
+ bool UpdateFlags) {
+ assert((BundleStart.getOpcode() == TargetOpcode::BUNDLE) &&
+ "Bundle start is not a bundle");
+ SmallVector<SlotIndex, 16> ToProcess;
+ const SlotIndex NewIndex = Indexes->insertMachineInstrInMaps(BundleStart);
+ auto BundleEnd = getBundleEnd(BundleStart.getIterator());
+
+ auto I = BundleStart.getIterator();
+ I++;
+ while (I != BundleEnd) {
+ if (!Indexes->hasIndex(*I))
+ continue;
+ SlotIndex OldIndex = Indexes->getInstructionIndex(*I, true);
+ ToProcess.push_back(OldIndex);
+ Indexes->removeMachineInstrFromMaps(*I, true);
+ I++;
+ }
+ for (SlotIndex OldIndex : ToProcess) {
+ HMEditor HME(*this, *MRI, *TRI, OldIndex, NewIndex, UpdateFlags);
+ HME.updateAllRanges(&BundleStart);
+ }
+
+ // Fix up dead defs
+ const SlotIndex Index = getInstructionIndex(BundleStart);
+ for (unsigned Idx = 0, E = BundleStart.getNumOperands(); Idx != E; ++Idx) {
+ MachineOperand &MO = BundleStart.getOperand(Idx);
+ if (!MO.isReg())
+ continue;
+ Register Reg = MO.getReg();
+ if (Reg.isVirtual() && hasInterval(Reg) && !MO.isUndef()) {
+ LiveInterval &LI = getInterval(Reg);
+ LiveQueryResult LRQ = LI.Query(Index);
+ if (LRQ.isDeadDef())
+ MO.setIsDead();
+ }
+ }
}
void LiveIntervals::repairOldRegInRange(const MachineBasicBlock::iterator Begin,
diff --git a/llvm/lib/CodeGen/SlotIndexes.cpp b/llvm/lib/CodeGen/SlotIndexes.cpp
index 6664b58eccf8..d2bfdc663edb 100644
--- a/llvm/lib/CodeGen/SlotIndexes.cpp
+++ b/llvm/lib/CodeGen/SlotIndexes.cpp
@@ -112,9 +112,10 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
return false;
}
-void SlotIndexes::removeMachineInstrFromMaps(MachineInstr &MI) {
- assert(!MI.isBundledWithPred() &&
- "Use removeSingleMachineInstrFromMaps() instread");
+void SlotIndexes::removeMachineInstrFromMaps(MachineInstr &MI,
+ bool AllowBundled) {
+ assert((AllowBundled || !MI.isBundledWithPred()) &&
+ "Use removeSingleMachineInstrFromMaps() instead");
Mi2IndexMap::iterator mi2iItr = mi2iMap.find(&MI);
if (mi2iItr == mi2iMap.end())
return;
@@ -141,7 +142,7 @@ void SlotIndexes::removeSingleMachineInstrFromMaps(MachineInstr &MI) {
// instruction.
if (MI.isBundledWithSucc()) {
// Only the first instruction of a bundle should have an index assigned.
- assert(!MI.isBundledWithPred() && "Should have first bundle isntruction");
+ assert(!MI.isBundledWithPred() && "Should be first bundle instruction");
MachineBasicBlock::instr_iterator Next = std::next(MI.getIterator());
MachineInstr &NextMI = *Next;
diff --git a/llvm/unittests/MI/LiveIntervalTest.cpp b/llvm/unittests/MI/LiveIntervalTest.cpp
index 835d3f91c66e..f0be9709332b 100644
--- a/llvm/unittests/MI/LiveIntervalTest.cpp
+++ b/llvm/unittests/MI/LiveIntervalTest.cpp
@@ -128,6 +128,27 @@ static void testHandleMove(MachineFunction &MF, LiveIntervals &LIS,
LIS.handleMove(FromInstr, true);
}
+/**
+ * Move instructions numbered \p From inclusive through instruction number
+ * \p To into a newly formed bundle and update affected liveness intervals
+ * with LiveIntervalAnalysis::handleMoveIntoNewBundle().
+ */
+static void testHandleMoveIntoNewBundle(MachineFunction &MF, LiveIntervals &LIS,
+ unsigned From, unsigned To,
+ unsigned BlockNum = 0) {
+ MachineInstr &FromInstr = getMI(MF, From, BlockNum);
+ MachineInstr &ToInstr = getMI(MF, To, BlockNum);
+ MachineBasicBlock &MBB = *FromInstr.getParent();
+ MachineBasicBlock::instr_iterator I = FromInstr.getIterator();
+
+ // Build bundle
+ finalizeBundle(MBB, I, std::next(ToInstr.getIterator()));
+
+ // Update LiveIntervals
+ MachineBasicBlock::instr_iterator BundleStart = std::prev(I);
+ LIS.handleMoveIntoNewBundle(*BundleStart, true);
+}
+
static void liveIntervalTest(StringRef MIRFunc, LiveIntervalTest T) {
LLVMContext Context;
std::unique_ptr<LLVMTargetMachine> TM = createTargetMachine();
@@ -462,6 +483,96 @@ TEST(LiveIntervalTest, TestMoveSubRegDefAcrossUseDefMulti) {
testHandleMove(MF, LIS, 4, 1, 1);
});
}
+
+TEST(LiveIntervalTest, BundleUse) {
+ liveIntervalTest(R"MIR(
+ %0 = IMPLICIT_DEF
+ S_NOP 0
+ S_NOP 0, implicit %0
+ S_NOP 0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 1, 2);
+ });
+}
+
+TEST(LiveIntervalTest, BundleDef) {
+ liveIntervalTest(R"MIR(
+ %0 = IMPLICIT_DEF
+ S_NOP 0
+ S_NOP 0, implicit %0
+ S_NOP 0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 0, 1);
+ });
+}
+
+TEST(LiveIntervalTest, BundleRedef) {
+ liveIntervalTest(R"MIR(
+ %0 = IMPLICIT_DEF
+ S_NOP 0
+ %0 = IMPLICIT_DEF implicit %0(tied-def 0)
+ S_NOP 0, implicit %0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 1, 2);
+ });
+}
+
+TEST(LiveIntervalTest, BundleInternalUse) {
+ liveIntervalTest(R"MIR(
+ %0 = IMPLICIT_DEF
+ S_NOP 0
+ S_NOP 0, implicit %0
+ S_NOP 0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 0, 2);
+ });
+}
+
+TEST(LiveIntervalTest, BundleUndefUse) {
+ liveIntervalTest(R"MIR(
+ %0 = IMPLICIT_DEF
+ S_NOP 0
+ S_NOP 0, implicit undef %0
+ S_NOP 0
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 1, 2);
+ });
+}
+
+TEST(LiveIntervalTest, BundleSubRegUse) {
+ liveIntervalTest(R"MIR(
+ successors: %bb.1, %bb.2
+ undef %0.sub0 = IMPLICIT_DEF
+ %0.sub1 = IMPLICIT_DEF
+ S_CBRANCH_VCCNZ %bb.2, implicit undef $vcc
+ S_BRANCH %bb.1
+ bb.1:
+ S_NOP 0
+ S_NOP 0, implicit %0.sub1
+ bb.2:
+ S_NOP 0, implicit %0.sub1
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 0, 1, 1);
+ });
+}
+
+TEST(LiveIntervalTest, BundleSubRegDef) {
+ liveIntervalTest(R"MIR(
+ successors: %bb.1, %bb.2
+ undef %0.sub0 = IMPLICIT_DEF
+ %0.sub1 = IMPLICIT_DEF
+ S_CBRANCH_VCCNZ %bb.2, implicit undef $vcc
+ S_BRANCH %bb.1
+ bb.1:
+ S_NOP 0
+ S_NOP 0, implicit %0.sub1
+ bb.2:
+ S_NOP 0, implicit %0.sub1
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+ testHandleMoveIntoNewBundle(MF, LIS, 0, 1, 0);
+ });
+}
+
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
initLLVM();
More information about the llvm-commits
mailing list