[llvm] 68c718c - Revert "[MachineOutliner][AArch64] NFC: Split MBBs into "outlinable ranges""
Jessica Paquette via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 23 10:36:47 PST 2022
Author: Jessica Paquette
Date: 2022-02-23T10:35:52-08:00
New Revision: 68c718c8f4b77dac87bdf7dfd9f2b14f3bac0592
URL: https://github.com/llvm/llvm-project/commit/68c718c8f4b77dac87bdf7dfd9f2b14f3bac0592
DIFF: https://github.com/llvm/llvm-project/commit/68c718c8f4b77dac87bdf7dfd9f2b14f3bac0592.diff
LOG: Revert "[MachineOutliner][AArch64] NFC: Split MBBs into "outlinable ranges""
This reverts commit d97f997eb79d91b2872ac13619f49cb3a7120781.
This commit was not NFC.
(See: https://reviews.llvm.org/rGd97f997eb79d91b2872ac13619f49cb3a7120781)
Added:
Modified:
llvm/include/llvm/CodeGen/TargetInstrInfo.h
llvm/lib/CodeGen/MachineOutliner.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index a3209af8b2352..12cd21617b0d4 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -1940,25 +1940,6 @@ class TargetInstrInfo : public MCInstrInfo {
virtual bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
unsigned &Flags) const;
- /// Optional target hook which partitions \p MBB into outlinable ranges for
- /// instruction mapping purposes. Each range is defined by two iterators:
- /// [start, end).
- ///
- /// Ranges are expected to be ordered top-down. That is, ranges closer to the
- /// top of the block should come before ranges closer to the end of the block.
- ///
- /// Ranges cannot overlap.
- ///
- /// If an entire block is mappable, then its range is [MBB.begin(), MBB.end())
- ///
- /// All instructions not present in an outlinable range are considered
- /// illegal.
- virtual SmallVector<
- std::pair<MachineBasicBlock::iterator, MachineBasicBlock::iterator>>
- getOutlinableRanges(MachineBasicBlock &MBB, unsigned &Flags) const {
- return {std::make_pair(MBB.begin(), MBB.end())};
- }
-
/// Insert a custom frame for outlined functions.
virtual void buildOutlinedFrame(MachineBasicBlock &MBB, MachineFunction &MF,
const outliner::OutlinedFunction &OF) const {
diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp
index e74264276d4c1..7ce655dce8e34 100644
--- a/llvm/lib/CodeGen/MachineOutliner.cpp
+++ b/llvm/lib/CodeGen/MachineOutliner.cpp
@@ -258,10 +258,6 @@ struct InstructionMapper {
if (!TII.isMBBSafeToOutlineFrom(MBB, Flags))
return;
- auto Ranges = TII.getOutlinableRanges(MBB, Flags);
- if (Ranges.empty())
- return;
-
// Store info for the MBB for later outlining.
MBBFlagsMap[&MBB] = Flags;
@@ -284,47 +280,34 @@ struct InstructionMapper {
std::vector<unsigned> UnsignedVecForMBB;
std::vector<MachineBasicBlock::iterator> InstrListForMBB;
- for (auto &Range : Ranges) {
- auto RangeStart = Range.first;
- auto RangeEnd = Range.second;
- // Everything outside of an outlinable range is illegal.
- for (; It != RangeStart; ++It)
+ for (MachineBasicBlock::iterator Et = MBB.end(); It != Et; ++It) {
+ // Keep track of where this instruction is in the module.
+ switch (TII.getOutliningType(It, Flags)) {
+ case InstrType::Illegal:
mapToIllegalUnsigned(It, CanOutlineWithPrevInstr, UnsignedVecForMBB,
InstrListForMBB);
- assert(It != MBB.end() && "Should still have instructions?");
- // `It` is now positioned at the beginning of a range of instructions
- // which may be outlinable. Check if each instruction is known to be safe.
- for (; It != RangeEnd; ++It) {
- // Keep track of where this instruction is in the module.
- switch (TII.getOutliningType(It, Flags)) {
- case InstrType::Illegal:
- mapToIllegalUnsigned(It, CanOutlineWithPrevInstr, UnsignedVecForMBB,
- InstrListForMBB);
- break;
-
- case InstrType::Legal:
- mapToLegalUnsigned(It, CanOutlineWithPrevInstr, HaveLegalRange,
- NumLegalInBlock, UnsignedVecForMBB,
- InstrListForMBB);
- break;
-
- case InstrType::LegalTerminator:
- mapToLegalUnsigned(It, CanOutlineWithPrevInstr, HaveLegalRange,
- NumLegalInBlock, UnsignedVecForMBB,
+ break;
+
+ case InstrType::Legal:
+ mapToLegalUnsigned(It, CanOutlineWithPrevInstr, HaveLegalRange,
+ NumLegalInBlock, UnsignedVecForMBB, InstrListForMBB);
+ break;
+
+ case InstrType::LegalTerminator:
+ mapToLegalUnsigned(It, CanOutlineWithPrevInstr, HaveLegalRange,
+ NumLegalInBlock, UnsignedVecForMBB, InstrListForMBB);
+ // The instruction also acts as a terminator, so we have to record that
+ // in the string.
+ mapToIllegalUnsigned(It, CanOutlineWithPrevInstr, UnsignedVecForMBB,
InstrListForMBB);
- // The instruction also acts as a terminator, so we have to record
- // that in the string.
- mapToIllegalUnsigned(It, CanOutlineWithPrevInstr, UnsignedVecForMBB,
- InstrListForMBB);
- break;
-
- case InstrType::Invisible:
- // Normally this is set by mapTo(Blah)Unsigned, but we just want to
- // skip this instruction. So, unset the flag here.
- ++NumInvisible;
- AddedIllegalLastTime = false;
- break;
- }
+ break;
+
+ case InstrType::Invisible:
+ // Normally this is set by mapTo(Blah)Unsigned, but we just want to
+ // skip this instruction. So, unset the flag here.
+ ++NumInvisible;
+ AddedIllegalLastTime = false;
+ break;
}
}
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index d160a33b529e2..84469dd257cab 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -6783,11 +6783,48 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo(
// Properties about candidate MBBs that hold for all of them.
unsigned FlagsSetInAll = 0xF;
+
+ // Compute liveness information for each candidate, and set FlagsSetInAll.
std::for_each(RepeatedSequenceLocs.begin(), RepeatedSequenceLocs.end(),
[&FlagsSetInAll](outliner::Candidate &C) {
FlagsSetInAll &= C.Flags;
});
+ // According to the AArch64 Procedure Call Standard, the following are
+ // undefined on entry/exit from a function call:
+ //
+ // * Registers x16, x17, (and thus w16, w17)
+ // * Condition codes (and thus the NZCV register)
+ //
+ // Because if this, we can't outline any sequence of instructions where
+ // one
+ // of these registers is live into/across it. Thus, we need to delete
+ // those
+ // candidates.
+ auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) {
+ // If the unsafe registers in this block are all dead, then we don't need
+ // to compute liveness here.
+ if (C.Flags & UnsafeRegsDead)
+ return false;
+ return C.isAnyUnavailableAcrossOrOutOfSeq(
+ {AArch64::W16, AArch64::W17, AArch64::NZCV}, TRI);
+ };
+
+ // Are there any candidates where those registers are live?
+ if (!(FlagsSetInAll & UnsafeRegsDead)) {
+ // Erase every candidate that violates the restrictions above. (It could be
+ // true that we have viable candidates, so it's not worth bailing out in
+ // the case that, say, 1 out of 20 candidates violate the restructions.)
+ llvm::erase_if(RepeatedSequenceLocs, CantGuaranteeValueAcrossCall);
+
+ // If the sequence doesn't have enough candidates left, then we're done.
+ if (RepeatedSequenceLocs.size() < 2)
+ return outliner::OutlinedFunction();
+ }
+
+ // At this point, we have only "safe" candidates to outline. Figure out
+ // frame + call instruction information.
+
unsigned LastInstrOpcode = RepeatedSequenceLocs[0].back()->getOpcode();
// Helper lambda which sets call information for every candidate.
@@ -6915,10 +6952,6 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo(
// Check if we have to save LR.
for (outliner::Candidate &C : RepeatedSequenceLocs) {
- bool LRAvailable =
- (C.Flags & MachineOutlinerMBBFlags::LRUnavailableSomewhere)
- ? C.isAvailableAcrossAndOutOfSeq(AArch64::LR, TRI)
- : true;
// If we have a noreturn caller, then we're going to be conservative and
// say that we have to save LR. If we don't have a ret at the end of the
// block, then we can't reason about liveness accurately.
@@ -6929,7 +6962,7 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo(
C.getMF()->getFunction().hasFnAttribute(Attribute::NoReturn);
// Is LR available? If so, we don't need a save.
- if (LRAvailable && !IsNoReturn) {
+ if (C.isAvailableAcrossAndOutOfSeq(AArch64::LR, TRI) && !IsNoReturn) {
NumBytesNoStackCalls += 4;
C.setCallInfo(MachineOutlinerNoLRSave, 4);
CandidatesWithoutStackFixups.push_back(C);
@@ -7101,88 +7134,72 @@ bool AArch64InstrInfo::isFunctionSafeToOutlineFrom(
return true;
}
-SmallVector<std::pair<MachineBasicBlock::iterator, MachineBasicBlock::iterator>>
-AArch64InstrInfo::getOutlinableRanges(MachineBasicBlock &MBB,
- unsigned &Flags) const {
+bool AArch64InstrInfo::isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
+ unsigned &Flags) const {
+ if (!TargetInstrInfo::isMBBSafeToOutlineFrom(MBB, Flags))
+ return false;
+ // Check if LR is available through all of the MBB. If it's not, then set
+ // a flag.
assert(MBB.getParent()->getRegInfo().tracksLiveness() &&
- "Must track liveness!");
- SmallVector<
- std::pair<MachineBasicBlock::iterator, MachineBasicBlock::iterator>>
- Ranges;
+ "Suitable Machine Function for outlining must track liveness");
+ LiveRegUnits LRU(getRegisterInfo());
- // The range [RangeBegin, RangeEnd).
- MachineBasicBlock::instr_iterator RangeEnd = MBB.instr_end();
- MachineBasicBlock::instr_iterator RangeBegin = RangeEnd;
- unsigned RangeLen = 0;
+ std::for_each(MBB.rbegin(), MBB.rend(),
+ [&LRU](MachineInstr &MI) { LRU.accumulate(MI); });
- // According to the AArch64 Procedure Call Standard, the following are
- // undefined on entry/exit from a function call:
- //
- // * Registers x16, x17, (and thus w16, w17)
- // * Condition codes (and thus the NZCV register)
- //
- // If any of these registers are used inside or live across an outlined
- // function, then they may be modified later, either by the compiler or
- // some other tool (like the linker).
- //
- // To avoid outlining in these situations, partition each block into ranges
- // where these registers are dead. We will only outline from those ranges.
- LiveRegUnits LRU(getRegisterInfo());
- auto AreAllUnsafeRegsDead = [&LRU]() {
- return LRU.available(AArch64::W16) && LRU.available(AArch64::W17) &&
- LRU.available(AArch64::NZCV);
- };
+ // Check if each of the unsafe registers are available...
+ bool W16AvailableInBlock = LRU.available(AArch64::W16);
+ bool W17AvailableInBlock = LRU.available(AArch64::W17);
+ bool NZCVAvailableInBlock = LRU.available(AArch64::NZCV);
- // We need to know if LR is live across an outlining boundary later on in
- // order to decide how we'll create the outlined call, frame, etc.
- //
- // It's pretty expensive to check this for *every candidate* within a block.
- // That's some potentially n^2 behaviour, since in the worst case, we'd need
- // to compute liveness from the end of the block for O(n) candidates within
- // the block.
- //
- // So, to improve the average case, let's keep track of liveness from the end
- // of the block to the beginning of *every outlinable range*. If we know that
- // LR is available in every range we could outline from, then we know that
- // we don't need to check liveness for any candidate within that range.
- bool LRAvailableEverywhere = true;
+ // If all of these are dead (and not live out), we know we don't have to check
+ // them later.
+ if (W16AvailableInBlock && W17AvailableInBlock && NZCVAvailableInBlock)
+ Flags |= MachineOutlinerMBBFlags::UnsafeRegsDead;
- // Compute liveness bottom-up.
+ // Now, add the live outs to the set.
LRU.addLiveOuts(MBB);
- for (auto &MI : make_range(MBB.instr_rbegin(), MBB.instr_rend())) {
- LRU.stepBackward(MI);
- // If we are in a range where all of the unsafe registers are dead, then
- // update the beginning of the range. Also try to precalculate some stuff
- // for getOutliningCandidateInfo.
- if (AreAllUnsafeRegsDead()) {
- if (MI.isCall())
- Flags |= MachineOutlinerMBBFlags::HasCalls;
- LRAvailableEverywhere &= LRU.available(AArch64::LR);
- RangeBegin = MI.getIterator();
- ++RangeLen;
- continue;
+
+ // If any of these registers is available in the MBB, but also a live out of
+ // the block, then we know outlining is unsafe.
+ if (W16AvailableInBlock && !LRU.available(AArch64::W16))
+ return false;
+ if (W17AvailableInBlock && !LRU.available(AArch64::W17))
+ return false;
+ if (NZCVAvailableInBlock && !LRU.available(AArch64::NZCV))
+ return false;
+
+ // Check if there's a call inside this MachineBasicBlock. If there is, then
+ // set a flag.
+ if (any_of(MBB, [](MachineInstr &MI) { return MI.isCall(); }))
+ Flags |= MachineOutlinerMBBFlags::HasCalls;
+
+ MachineFunction *MF = MBB.getParent();
+
+ // In the event that we outline, we may have to save LR. If there is an
+ // available register in the MBB, then we'll always save LR there. Check if
+ // this is true.
+ bool CanSaveLR = false;
+ const AArch64RegisterInfo *ARI = static_cast<const AArch64RegisterInfo *>(
+ MF->getSubtarget().getRegisterInfo());
+
+ // Check if there is an available register across the sequence that we can
+ // use.
+ for (unsigned Reg : AArch64::GPR64RegClass) {
+ if (!ARI->isReservedReg(*MF, Reg) && Reg != AArch64::LR &&
+ Reg != AArch64::X16 && Reg != AArch64::X17 && LRU.available(Reg)) {
+ CanSaveLR = true;
+ break;
}
- // At least one unsafe register is not dead. We do not want to outline at
- // this point. If it is long enough to outline from, save the range
- // [RangeBegin, RangeEnd).
- if (RangeLen > 1)
- Ranges.push_back(std::make_pair(RangeBegin, RangeEnd));
- // Start a new range where RangeEnd is the first known unsafe point.
- RangeLen = 0;
- RangeBegin = MI.getIterator();
- RangeEnd = MI.getIterator();
- }
- // Above loop misses the last (or only) range.
- if (AreAllUnsafeRegsDead() && RangeLen > 1)
- Ranges.push_back(std::make_pair(RangeBegin, RangeEnd));
- if (Ranges.empty())
- return Ranges;
- // We found the ranges bottom-up. Mapping expects the top-down. Reverse
- // the order.
- std::reverse(Ranges.begin(), Ranges.end());
- if (!LRAvailableEverywhere)
+ }
+
+ // Check if we have a register we can save LR to, and if LR was used
+ // somewhere. If both of those things are true, then we need to evaluate the
+ // safety of outlining stack instructions later.
+ if (!CanSaveLR && !LRU.available(AArch64::LR))
Flags |= MachineOutlinerMBBFlags::LRUnavailableSomewhere;
- return Ranges;
+
+ return true;
}
outliner::InstrType
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index 677e1443191cd..55b1813f0b301 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -280,11 +280,10 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
bool OutlineFromLinkOnceODRs) const override;
outliner::OutlinedFunction getOutliningCandidateInfo(
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const override;
- outliner::InstrType getOutliningType(MachineBasicBlock::iterator &MIT,
- unsigned Flags) const override;
- SmallVector<
- std::pair<MachineBasicBlock::iterator, MachineBasicBlock::iterator>>
- getOutlinableRanges(MachineBasicBlock &MBB, unsigned &Flags) const override;
+ outliner::InstrType
+ getOutliningType(MachineBasicBlock::iterator &MIT, unsigned Flags) const override;
+ bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
+ unsigned &Flags) const override;
void buildOutlinedFrame(MachineBasicBlock &MBB, MachineFunction &MF,
const outliner::OutlinedFunction &OF) const override;
MachineBasicBlock::iterator
More information about the llvm-commits
mailing list