maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

CodeGen: Give MachineBasicBlock::reverse_iterator a handle to the current MI 

Now that MachineBasicBlock::reverse_instr_iterator knows when it's at
the end (since r281168 and r281170), implement
MachineBasicBlock::reverse_iterator directly on top of an
ilist::reverse_iterator by adding an IsReverse template parameter to
MachineInstrBundleIterator.  This replaces another hard-to-reason-about
use of std::reverse_iterator on list iterators, matching the changes for
ilist::reverse_iterator from r280032 (see the "out of scope" section at
the end of that commit message).  MachineBasicBlock::reverse_iterator
now has a handle to the current node and has obvious invalidation
semantics.

r280032 has a more detailed explanation of how list-style reverse
iterators (invalidated when the pointed-at node is deleted) are
different from vector-style reverse iterators like std::reverse_iterator
(invalidated on every operation).  A great motivating example is this
commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp.

Note: If your out-of-tree backend deletes instructions while iterating
on a MachineBasicBlock::reverse_iterator or converts between
MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator,
you'll need to update your code in similar ways to r280032.  The
following table might help:

                  [Old]              ==>             [New]
        delete &*RI, RE = end()                   delete &*RI++
        RI->erase(), RE = end()                   RI++->erase()
      reverse_iterator(I)                 std::prev(I).getReverse()
      reverse_iterator(I)                          ++I.getReverse()
    --reverse_iterator(I)                            I.getReverse()
      reverse_iterator(std::next(I))                 I.getReverse()
                RI.base()                std::prev(RI).getReverse()
                RI.base()                         ++RI.getReverse()
              --RI.base()                           RI.getReverse()
     std::next(RI).base()                           RI.getReverse()

(For more details, have a look at r280032.)

llvm-svn: 281172
This commit is contained in:
Duncan P. N. Exon Smith 2016-09-11 18:51:28 +00:00
parent e98bc7af8b
commit 1872096f1e
14 changed files with 147 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
llvm/include/llvm/CodeGen/MachineBasicBlock.h
View File

@ -155,9 +155,9 @@ public:
typedef MachineInstrBundleIterator<MachineInstr> iterator;
typedef MachineInstrBundleIterator<const MachineInstr> const_iterator;
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
typedef std::reverse_iterator<iterator> reverse_iterator;
typedef MachineInstrBundleIterator<MachineInstr, true> reverse_iterator;
typedef MachineInstrBundleIterator<const MachineInstr, true>
const_reverse_iterator;
unsigned size() const { return (unsigned)Insts.size(); }
bool empty() const { return Insts.empty(); }
@ -192,13 +192,15 @@ public:
const_iterator begin() const { return instr_begin(); }
iterator end () { return instr_end(); }
const_iterator end () const { return instr_end(); }
reverse_iterator rbegin() { return reverse_iterator(end()); }
const_reverse_iterator rbegin() const {
return const_reverse_iterator(end());
reverse_iterator rbegin() {
return reverse_iterator::getAtBundleBegin(instr_rbegin());
}
reverse_iterator rend() { return reverse_iterator(begin()); }
const_reverse_iterator rbegin() const {
return const_reverse_iterator::getAtBundleBegin(instr_rbegin());
}
reverse_iterator rend() { return reverse_iterator(instr_rend()); }
const_reverse_iterator rend() const {
return const_reverse_iterator(begin());
return const_reverse_iterator(instr_rend());
}
/// Support for MachineInstr::getNextNode().

97
llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h
View File

@ -19,23 +19,92 @@
namespace llvm {
template <class T> struct MachineInstrBundleIteratorTraits {
template <class T, bool IsReverse> struct MachineInstrBundleIteratorTraits;
template <class T> struct MachineInstrBundleIteratorTraits<T, false> {
typedef simple_ilist<T, ilist_sentinel_tracking<true>> list_type;
typedef typename list_type::iterator instr_iterator;
typedef typename list_type::iterator nonconst_instr_iterator;
typedef typename list_type::const_iterator const_instr_iterator;
};
template <class T> struct MachineInstrBundleIteratorTraits<const T> {
template <class T> struct MachineInstrBundleIteratorTraits<T, true> {
typedef simple_ilist<T, ilist_sentinel_tracking<true>> list_type;
typedef typename list_type::reverse_iterator instr_iterator;
typedef typename list_type::reverse_iterator nonconst_instr_iterator;
typedef typename list_type::const_reverse_iterator const_instr_iterator;
};
template <class T> struct MachineInstrBundleIteratorTraits<const T, false> {
typedef simple_ilist<T, ilist_sentinel_tracking<true>> list_type;
typedef typename list_type::const_iterator instr_iterator;
typedef typename list_type::iterator nonconst_instr_iterator;
typedef typename list_type::const_iterator const_instr_iterator;
};
template <class T> struct MachineInstrBundleIteratorTraits<const T, true> {
typedef simple_ilist<T, ilist_sentinel_tracking<true>> list_type;
typedef typename list_type::const_reverse_iterator instr_iterator;
typedef typename list_type::reverse_iterator nonconst_instr_iterator;
typedef typename list_type::const_reverse_iterator const_instr_iterator;
};
template <bool IsReverse> struct MachineInstrBundleIteratorHelper;
template <> struct MachineInstrBundleIteratorHelper<false> {
/// Get the beginning of the current bundle.
template <class Iterator> static Iterator getBundleBegin(Iterator I) {
if (!I.isEnd())
while (I->isBundledWithPred())
--I;
return I;
}
/// Get the final node of the current bundle.
template <class Iterator> static Iterator getBundleFinal(Iterator I) {
if (!I.isEnd())
while (I->isBundledWithSucc())
++I;
return I;
}
/// Increment forward ilist iterator.
template <class Iterator> static void increment(Iterator &I) {
I = std::next(getBundleFinal(I));
}
/// Decrement forward ilist iterator.
template <class Iterator> static void decrement(Iterator &I) {
I = getBundleBegin(std::prev(I));
}
};
template <> struct MachineInstrBundleIteratorHelper<true> {
/// Get the beginning of the current bundle.
template <class Iterator> static Iterator getBundleBegin(Iterator I) {
return MachineInstrBundleIteratorHelper<false>::getBundleBegin(
I.getReverse())
.getReverse();
}
/// Get the final node of the current bundle.
template <class Iterator> static Iterator getBundleFinal(Iterator I) {
return MachineInstrBundleIteratorHelper<false>::getBundleFinal(
I.getReverse())
.getReverse();
}
/// Increment reverse ilist iterator.
template <class Iterator> static void increment(Iterator &I) {
I = getBundleBegin(std::next(I));
}
/// Decrement reverse ilist iterator.
template <class Iterator> static void decrement(Iterator &I) {
I = std::prev(getBundleFinal(I));
}
};
/// MachineBasicBlock iterator that automatically skips over MIs that are
/// inside bundles (i.e. walk top level MIs only).
template <typename Ty> class MachineInstrBundleIterator {
typedef MachineInstrBundleIteratorTraits<Ty> Traits;
template <typename Ty, bool IsReverse = false>
class MachineInstrBundleIterator : MachineInstrBundleIteratorHelper<IsReverse> {
typedef MachineInstrBundleIteratorTraits<Ty, IsReverse> Traits;
typedef typename Traits::instr_iterator instr_iterator;
instr_iterator MII;
@ -53,8 +122,9 @@ private:
typedef typename Traits::nonconst_instr_iterator nonconst_instr_iterator;
typedef typename Traits::const_instr_iterator const_instr_iterator;
typedef MachineInstrBundleIterator<
typename nonconst_instr_iterator::value_type>
typename nonconst_instr_iterator::value_type, IsReverse>
nonconst_iterator;
typedef MachineInstrBundleIterator<Ty, !IsReverse> reverse_iterator;
public:
MachineInstrBundleIterator(instr_iterator MI) : MII(MI) {
@ -77,12 +147,17 @@ public:
// Template allows conversion from const to nonconst.
template <class OtherTy>
MachineInstrBundleIterator(
const MachineInstrBundleIterator<OtherTy> &I,
const MachineInstrBundleIterator<OtherTy, IsReverse> &I,
typename std::enable_if<std::is_convertible<OtherTy *, Ty *>::value,
void *>::type = nullptr)
: MII(I.getInstrIterator()) {}
MachineInstrBundleIterator() : MII(nullptr) {}
/// Get the bundle iterator for the given instruction's bundle.
static MachineInstrBundleIterator getAtBundleBegin(instr_iterator MI) {
return MachineInstrBundleIteratorHelper<IsReverse>::getBundleBegin(MI);
}
reference operator*() const { return *MII; }
pointer operator->() const { return &operator*(); }
@ -161,15 +236,11 @@ public:
// Increment and decrement operators...
MachineInstrBundleIterator &operator--() {
do
--MII;
while (MII->isBundledWithPred());
this->decrement(MII);
return *this;
}
MachineInstrBundleIterator &operator++() {
while (MII->isBundledWithSucc())
++MII;
++MII;
this->increment(MII);
return *this;
}
MachineInstrBundleIterator operator--(int) {
@ -186,6 +257,8 @@ public:
instr_iterator getInstrIterator() const { return MII; }
nonconst_iterator getNonConstIterator() const { return MII.getNonConst(); }
reverse_iterator getReverse() const { return MII.getReverse(); }
};
} // end namespace llvm

4
llvm/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp
View File

@ -134,8 +134,8 @@ static const MachineInstr *getFirstEpilogueInst(const MachineBasicBlock &MBB) {
// as the return instruction.
DebugLoc LastLoc = LastMI->getDebugLoc();
auto Res = LastMI;
for (MachineBasicBlock::const_reverse_iterator I(std::next(LastMI)),
E = MBB.rend();
for (MachineBasicBlock::const_reverse_iterator I = LastMI.getReverse(),
E = MBB.rend();
I != E; ++I) {
if (I->getDebugLoc() != LastLoc)
return &*Res;

9
llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
View File

@ -122,7 +122,7 @@ bool DeadMachineInstructionElim::runOnMachineFunction(MachineFunction &MF) {
// liveness as we go.
for (MachineBasicBlock::reverse_iterator MII = MBB.rbegin(),
MIE = MBB.rend(); MII != MIE; ) {
MachineInstr *MI = &*MII;
MachineInstr *MI = &*MII++;
// If the instruction is dead, delete it!
if (isDead(MI)) {
@ -133,9 +133,6 @@ bool DeadMachineInstructionElim::runOnMachineFunction(MachineFunction &MF) {
MI->eraseFromParentAndMarkDBGValuesForRemoval();
AnyChanges = true;
++NumDeletes;
MIE = MBB.rend();
// MII is now pointing to the next instruction to process,
// so don't increment it.
continue;
}
@ -169,10 +166,6 @@ bool DeadMachineInstructionElim::runOnMachineFunction(MachineFunction &MF) {
}
}
}
// We didn't delete the current instruction, so increment MII to
// the next one.
++MII;
}
}

3
llvm/lib/CodeGen/IfConversion.cpp
View File

@ -1802,8 +1802,7 @@ bool IfConverter::IfConvertDiamondCommon(
// This is everything used+live in BB2 after the duplicated instructions. We
// can compute this set by simulating liveness backwards from the end of BB2.
DontKill.init(TRI);
for (const MachineInstr &MI :
make_range(MBB2.rbegin(), MachineBasicBlock::reverse_iterator(DI2)))
for (const MachineInstr &MI : make_range(MBB2.rbegin(), ++DI2.getReverse()))
DontKill.stepBackward(MI);
for (const MachineInstr &MI : make_range(MBB1.begin(), DI1)) {

6
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
View File

@ -920,9 +920,9 @@ static bool areCFlagsAccessedBetweenInstrs(
return true;
// From must be above To.
assert(std::find_if(MachineBasicBlock::reverse_iterator(To),
To->getParent()->rend(), [From](MachineInstr &MI) {
return MachineBasicBlock::iterator(MI) == From;
assert(std::find_if(++To.getReverse(), To->getParent()->rend(),
[From](MachineInstr &MI) {
return MI.getIterator() == From;
}) != To->getParent()->rend());
// We iterate backward starting \p To until we hit \p From.

2
llvm/lib/Target/AMDGPU/R600InstrInfo.cpp
View File

@ -726,7 +726,7 @@ MachineBasicBlock::iterator FindLastAluClause(MachineBasicBlock &MBB) {
It != E; ++It) {
if (It->getOpcode() == AMDGPU::CF_ALU ||
It->getOpcode() == AMDGPU::CF_ALU_PUSH_BEFORE)
return std::prev(It.base());
return It.getReverse();
}
return MBB.end();
}

11
llvm/lib/Target/ARM/MLxExpansionPass.cpp
View File

@ -334,18 +334,15 @@ bool MLxExpansion::ExpandFPMLxInstructions(MachineBasicBlock &MBB) {
unsigned Skip = 0;
MachineBasicBlock::reverse_iterator MII = MBB.rbegin(), E = MBB.rend();
while (MII != E) {
MachineInstr *MI = &*MII;
MachineInstr *MI = &*MII++;
if (MI->isPosition() || MI->isImplicitDef() || MI->isCopy()) {
++MII;
if (MI->isPosition() || MI->isImplicitDef() || MI->isCopy())
continue;
}
const MCInstrDesc &MCID = MI->getDesc();
if (MI->isBarrier()) {
clearStack();
Skip = 0;
++MII;
continue;
}
@ -365,13 +362,9 @@ bool MLxExpansion::ExpandFPMLxInstructions(MachineBasicBlock &MBB) {
pushStack(MI);
else {
ExpandFPMLxInstruction(MBB, MI, MulOpc, AddSubOpc, NegAcc, HasLane);
E = MBB.rend(); // May have changed if MI was the 1st instruction.
Changed = true;
continue;
}
}
++MII;
}
return Changed;

2
llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
View File

@ -983,7 +983,7 @@ void HexagonInstrInfo::loadRegFromStackSlot(
static void getLiveRegsAt(LivePhysRegs &Regs, const MachineInstr &MI) {
const MachineBasicBlock &B = *MI.getParent();
Regs.addLiveOuts(B);
auto E = MachineBasicBlock::const_reverse_iterator(MI.getIterator());
auto E = ++MachineBasicBlock::const_iterator(MI.getIterator()).getReverse();
for (auto I = B.rbegin(); I != E; ++I)
Regs.stepBackward(*I);
}

23
llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
View File

@ -242,7 +242,7 @@ namespace {
/// This function searches in the backward direction for an instruction that
/// can be moved to the delay slot. Returns true on success.
bool searchBackward(MachineBasicBlock &MBB, Iter Slot) const;
bool searchBackward(MachineBasicBlock &MBB, MachineInstr &Slot) const;
/// This function searches MBB in the forward direction for an instruction
/// that can be moved to the delay slot. Returns true on success.
@ -594,7 +594,7 @@ bool Filler::runOnMachineBasicBlock(MachineBasicBlock &MBB) {
if (MipsCompactBranchPolicy.getValue() != CB_Always ||
!TII->getEquivalentCompactForm(I)) {
if (searchBackward(MBB, I)) {
if (searchBackward(MBB, *I)) {
Filled = true;
} else if (I->isTerminator()) {
if (searchSuccBBs(MBB, I)) {
@ -659,8 +659,6 @@ template<typename IterTy>
bool Filler::searchRange(MachineBasicBlock &MBB, IterTy Begin, IterTy End,
RegDefsUses &RegDU, InspectMemInstr& IM, Iter Slot,
IterTy &Filler) const {
bool IsReverseIter = std::is_convertible<IterTy, ReverseIter>::value;
for (IterTy I = Begin; I != End;) {
IterTy CurrI = I;
++I;
@ -677,12 +675,6 @@ bool Filler::searchRange(MachineBasicBlock &MBB, IterTy Begin, IterTy End,
if (CurrI->isKill()) {
CurrI->eraseFromParent();
// This special case is needed for reverse iterators, because when we
// erase an instruction, the iterators are updated to point to the next
// instruction.
if (IsReverseIter && I != End)
I = CurrI;
continue;
}
@ -722,7 +714,7 @@ bool Filler::searchRange(MachineBasicBlock &MBB, IterTy Begin, IterTy End,
return false;
}
bool Filler::searchBackward(MachineBasicBlock &MBB, Iter Slot) const {
bool Filler::searchBackward(MachineBasicBlock &MBB, MachineInstr &Slot) const {
if (DisableBackwardSearch)
return false;
@ -731,14 +723,15 @@ bool Filler::searchBackward(MachineBasicBlock &MBB, Iter Slot) const {
MemDefsUses MemDU(Fn->getDataLayout(), &Fn->getFrameInfo());
ReverseIter Filler;
RegDU.init(*Slot);
RegDU.init(Slot);
if (!searchRange(MBB, ReverseIter(Slot), MBB.rend(), RegDU, MemDU, Slot,
MachineBasicBlock::iterator SlotI = Slot;
if (!searchRange(MBB, ++SlotI.getReverse(), MBB.rend(), RegDU, MemDU, Slot,
Filler))
return false;
MBB.splice(std::next(Slot), &MBB, std::next(Filler).base());
MIBundleBuilder(MBB, Slot, std::next(Slot, 2));
MBB.splice(std::next(SlotI), &MBB, Filler.getReverse());
MIBundleBuilder(MBB, SlotI, std::next(SlotI, 2));
++UsefulSlots;
return true;
}

8
llvm/lib/Target/Mips/MipsInstrInfo.cpp
View File

@ -147,14 +147,16 @@ unsigned MipsInstrInfo::InsertBranch(MachineBasicBlock &MBB,
unsigned MipsInstrInfo::RemoveBranch(MachineBasicBlock &MBB) const {
MachineBasicBlock::reverse_iterator I = MBB.rbegin(), REnd = MBB.rend();
MachineBasicBlock::reverse_iterator FirstBr;
unsigned removed;
// Skip all the debug instructions.
while (I != REnd && I->isDebugValue())
++I;
FirstBr = I;
if (I == REnd)
return 0;
MachineBasicBlock::iterator FirstBr = ++I.getReverse();
// Up to 2 branches are removed.
// Note that indirect branches are not removed.
@ -162,7 +164,7 @@ unsigned MipsInstrInfo::RemoveBranch(MachineBasicBlock &MBB) const {
if (!getAnalyzableBrOpc(I->getOpcode()))
break;
MBB.erase(I.base(), FirstBr.base());
MBB.erase((--I).getReverse(), FirstBr);
return removed;
}

4
llvm/lib/Target/Mips/MipsLongBranch.cpp
View File

@ -157,7 +157,7 @@ void MipsLongBranch::splitMBB(MachineBasicBlock *MBB) {
MBB->addSuccessor(Tgt);
MF->insert(std::next(MachineFunction::iterator(MBB)), NewMBB);
NewMBB->splice(NewMBB->end(), MBB, (++LastBr).base(), MBB->end());
NewMBB->splice(NewMBB->end(), MBB, LastBr.getReverse(), MBB->end());
}
// Fill MBBInfos.
@ -187,7 +187,7 @@ void MipsLongBranch::initMBBInfo() {
if ((Br != End) && !Br->isIndirectBranch() &&
(Br->isConditionalBranch() || (Br->isUnconditionalBranch() && IsPIC)))
MBBInfos[I].Br = &*(++Br).base();
MBBInfos[I].Br = &*Br;
}
}

9
llvm/lib/Target/X86/X86InstrInfo.cpp
View File

@ -5349,9 +5349,9 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,
// If the definition is in this basic block, RE points to the definition;
// otherwise, RE is the rend of the basic block.
MachineBasicBlock::reverse_iterator
RI = MachineBasicBlock::reverse_iterator(I),
RI = ++I.getReverse(),
RE = CmpInstr.getParent() == MI->getParent()
? MachineBasicBlock::reverse_iterator(++Def) /* points to MI */
? Def.getReverse() /* points to MI */
: CmpInstr.getParent()->rend();
MachineInstr *Movr0Inst = nullptr;
for (; RI != RE; ++RI) {
@ -5497,9 +5497,8 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,
if (Movr0Inst) {
// Look backwards until we find a def that doesn't use the current EFLAGS.
Def = Sub;
MachineBasicBlock::reverse_iterator
InsertI = MachineBasicBlock::reverse_iterator(++Def),
InsertE = Sub->getParent()->rend();
MachineBasicBlock::reverse_iterator InsertI = Def.getReverse(),
InsertE = Sub->getParent()->rend();
for (; InsertI != InsertE; ++InsertI) {
MachineInstr *Instr = &*InsertI;
if (!Instr->readsRegister(X86::EFLAGS, TRI) &&

22
llvm/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp
View File

@ -22,25 +22,47 @@ struct MyBundledInstr
};
typedef MachineInstrBundleIterator<MyBundledInstr> bundled_iterator;
typedef MachineInstrBundleIterator<const MyBundledInstr> const_bundled_iterator;
typedef MachineInstrBundleIterator<MyBundledInstr, true>
reverse_bundled_iterator;
typedef MachineInstrBundleIterator<const MyBundledInstr, true>
const_reverse_bundled_iterator;
#ifdef GTEST_HAS_DEATH_TEST
#ifndef NDEBUG
TEST(MachineInstrBundleIteratorTest, CheckForBundles) {
MyBundledInstr MBI;
auto I = MBI.getIterator();
auto RI = I.getReverse();
// Confirm that MBI is always considered bundled.
EXPECT_TRUE(MBI.isBundledWithPred());
EXPECT_TRUE(MBI.isBundledWithSucc());
// Confirm that iterators check in their constructor for bundled iterators.
EXPECT_DEATH((void)static_cast<bundled_iterator>(I),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<bundled_iterator>(&MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_bundled_iterator>(I),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_bundled_iterator>(&MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<reverse_bundled_iterator>(RI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<reverse_bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<reverse_bundled_iterator>(&MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_reverse_bundled_iterator>(RI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_reverse_bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_reverse_bundled_iterator>(&MBI),
"not legal to initialize");
}
#endif
#endif