diff --git a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelect.h b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelect.h index 8017f09aa3c8b..a2f06e21a700e 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelect.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelect.h @@ -20,6 +20,8 @@ namespace llvm { +class InstructionSelector; +class GISelKnownBits; class BlockFrequencyInfo; class ProfileSummaryInfo; @@ -53,12 +55,20 @@ class InstructionSelect : public MachineFunctionPass { char &PassID = ID); bool runOnMachineFunction(MachineFunction &MF) override; + bool selectMachineFunction(MachineFunction &MF); + void setInstructionSelector(InstructionSelector *NewISel) { ISel = NewISel; } protected: + class MIIteratorMaintainer; + + InstructionSelector *ISel = nullptr; + GISelKnownBits *KB = nullptr; BlockFrequencyInfo *BFI = nullptr; ProfileSummaryInfo *PSI = nullptr; CodeGenOptLevel OptLevel = CodeGenOptLevel::None; + + bool selectInstr(MachineInstr &MI); }; } // End namespace llvm. diff --git a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h index 8331cb58a0991..aaba56ee11251 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h @@ -32,12 +32,9 @@ class InstructionSelector : public GIMatchTableExecutor { /// !isPreISelGenericOpcode(I.getOpcode()) virtual bool select(MachineInstr &I) = 0; - void setTargetPassConfig(const TargetPassConfig *T) { TPC = T; } - - void setRemarkEmitter(MachineOptimizationRemarkEmitter *M) { MORE = M; } - -protected: + // FIXME: Eliminate dependency on TargetPassConfig for NewPM transition const TargetPassConfig *TPC = nullptr; + MachineOptimizationRemarkEmitter *MORE = nullptr; }; } // namespace llvm diff --git a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp index 9a27728dcb4dd..8c0bb85fd0771 100644 --- a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp +++ b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp @@ -12,8 +12,10 @@ #include "llvm/CodeGen/GlobalISel/InstructionSelect.h" #include "llvm/ADT/PostOrderIterator.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/SetVector.h" #include "llvm/Analysis/LazyBlockFrequencyInfo.h" #include "llvm/Analysis/ProfileSummaryInfo.h" +#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h" #include "llvm/CodeGen/GlobalISel/GISelKnownBits.h" #include "llvm/CodeGen/GlobalISel/InstructionSelector.h" #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h" @@ -65,6 +67,52 @@ INITIALIZE_PASS_END(InstructionSelect, DEBUG_TYPE, InstructionSelect::InstructionSelect(CodeGenOptLevel OL, char &PassID) : MachineFunctionPass(PassID), OptLevel(OL) {} +/// This class observes instruction insertions/removals. +/// InstructionSelect stores an iterator of the instruction prior to the one +/// that is currently being selected to determine which instruction to select +/// next. Previously this meant that selecting multiple instructions at once was +/// illegal behavior due to potential invalidation of this iterator. This is +/// a non-obvious limitation for selector implementers. Therefore, to allow +/// deletion of arbitrary instructions, we detect this case and continue +/// selection with the predecessor of the deleted instruction. +class InstructionSelect::MIIteratorMaintainer + : public MachineFunction::Delegate { +#ifndef NDEBUG + SmallSetVector CreatedInstrs; +#endif +public: + MachineBasicBlock::reverse_iterator MII; + + void MF_HandleInsertion(MachineInstr &MI) override { + LLVM_DEBUG(dbgs() << "Creating: " << MI; CreatedInstrs.insert(&MI)); + } + + void MF_HandleRemoval(MachineInstr &MI) override { + LLVM_DEBUG(dbgs() << "Erasing: " << MI; CreatedInstrs.remove(&MI)); + if (MII.getInstrIterator().getNodePtr() == &MI) { + // If the iterator points to the MI that will be erased (i.e. the MI prior + // to the MI that is currently being selected), the iterator would be + // invalidated. Continue selection with its predecessor. + ++MII; + LLVM_DEBUG(dbgs() << "Instruction removal updated iterator.\n"); + } + } + + void reportFullyCreatedInstrs() { + LLVM_DEBUG({ + if (CreatedInstrs.empty()) { + dbgs() << "Created no instructions.\n"; + } else { + dbgs() << "Created:\n"; + for (const auto *MI : CreatedInstrs) { + dbgs() << " " << *MI; + } + CreatedInstrs.clear(); + } + }); + } +}; + void InstructionSelect::getAnalysisUsage(AnalysisUsage &AU) const { AU.addRequired(); AU.addRequired(); @@ -84,31 +132,36 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) { MachineFunctionProperties::Property::FailedISel)) return false; - LLVM_DEBUG(dbgs() << "Selecting function: " << MF.getName() << '\n'); - - const TargetPassConfig &TPC = getAnalysis(); - InstructionSelector *ISel = MF.getSubtarget().getInstructionSelector(); - ISel->setTargetPassConfig(&TPC); + ISel = MF.getSubtarget().getInstructionSelector(); + ISel->TPC = &getAnalysis(); + // FIXME: Properly override OptLevel in TargetMachine. See OptLevelChanger CodeGenOptLevel OldOptLevel = OptLevel; auto RestoreOptLevel = make_scope_exit([=]() { OptLevel = OldOptLevel; }); OptLevel = MF.getFunction().hasOptNone() ? CodeGenOptLevel::None : MF.getTarget().getOptLevel(); - GISelKnownBits *KB = &getAnalysis().get(MF); + KB = &getAnalysis().get(MF); if (OptLevel != CodeGenOptLevel::None) { PSI = &getAnalysis().getPSI(); if (PSI && PSI->hasProfileSummary()) BFI = &getAnalysis().getBFI(); } - CodeGenCoverage CoverageInfo; + return selectMachineFunction(MF); +} + +bool InstructionSelect::selectMachineFunction(MachineFunction &MF) { + LLVM_DEBUG(dbgs() << "Selecting function: " << MF.getName() << '\n'); assert(ISel && "Cannot work without InstructionSelector"); + + const TargetPassConfig &TPC = *ISel->TPC; + CodeGenCoverage CoverageInfo; ISel->setupMF(MF, KB, &CoverageInfo, PSI, BFI); // An optimization remark emitter. Used to report failures. MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr); - ISel->setRemarkEmitter(&MORE); + ISel->MORE = &MORE; // FIXME: There are many other MF/MFI fields we need to initialize. @@ -131,80 +184,36 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) { // Keep track of selected blocks, so we can delete unreachable ones later. DenseSet SelectedBlocks; - for (MachineBasicBlock *MBB : post_order(&MF)) { - ISel->CurMBB = MBB; - SelectedBlocks.insert(MBB); - if (MBB->empty()) - continue; - - // Select instructions in reverse block order. We permit erasing so have - // to resort to manually iterating and recognizing the begin (rend) case. - bool ReachedBegin = false; - for (auto MII = std::prev(MBB->end()), Begin = MBB->begin(); - !ReachedBegin;) { -#ifndef NDEBUG - // Keep track of the insertion range for debug printing. - const auto AfterIt = std::next(MII); -#endif - // Select this instruction. - MachineInstr &MI = *MII; - - // And have our iterator point to the next instruction, if there is one. - if (MII == Begin) - ReachedBegin = true; - else - --MII; - - LLVM_DEBUG(dbgs() << "Selecting: \n " << MI); - - // We could have folded this instruction away already, making it dead. - // If so, erase it. - if (isTriviallyDead(MI, MRI)) { - LLVM_DEBUG(dbgs() << "Is dead; erasing.\n"); - salvageDebugInfo(MRI, MI); - MI.eraseFromParent(); - continue; - } - - // Eliminate hints or G_CONSTANT_FOLD_BARRIER. - if (isPreISelGenericOptimizationHint(MI.getOpcode()) || - MI.getOpcode() == TargetOpcode::G_CONSTANT_FOLD_BARRIER) { - auto [DstReg, SrcReg] = MI.getFirst2Regs(); - - // At this point, the destination register class of the op may have - // been decided. - // - // Propagate that through to the source register. - const TargetRegisterClass *DstRC = MRI.getRegClassOrNull(DstReg); - if (DstRC) - MRI.setRegClass(SrcReg, DstRC); - assert(canReplaceReg(DstReg, SrcReg, MRI) && - "Must be able to replace dst with src!"); - MI.eraseFromParent(); - MRI.replaceRegWith(DstReg, SrcReg); - continue; - } - - if (MI.getOpcode() == TargetOpcode::G_INVOKE_REGION_START) { - MI.eraseFromParent(); - continue; - } - - if (!ISel->select(MI)) { - // FIXME: It would be nice to dump all inserted instructions. It's - // not obvious how, esp. considering select() can insert after MI. - reportGISelFailure(MF, TPC, MORE, "gisel-select", "cannot select", MI); - return false; + { + // Observe IR insertions and removals during selection. + // We only install a MachineFunction::Delegate instead of a + // GISelChangeObserver, because we do not want notifications about changed + // instructions. This prevents significant compile-time regressions from + // e.g. constrainOperandRegClass(). + MIIteratorMaintainer MIIMaintainer; + RAIIDelegateInstaller DelInstaller(MF, &MIIMaintainer); + + for (MachineBasicBlock *MBB : post_order(&MF)) { + ISel->CurMBB = MBB; + SelectedBlocks.insert(MBB); + + // Select instructions in reverse block order. + MIIMaintainer.MII = MBB->rbegin(); + for (auto End = MBB->rend(); MIIMaintainer.MII != End;) { + MachineInstr &MI = *MIIMaintainer.MII; + // Increment early to skip instructions inserted by select(). + ++MIIMaintainer.MII; + + LLVM_DEBUG(dbgs() << "\nSelect: " << MI); + if (!selectInstr(MI)) { + LLVM_DEBUG(dbgs() << "Selection failed!\n"; + MIIMaintainer.reportFullyCreatedInstrs()); + reportGISelFailure(MF, TPC, MORE, "gisel-select", "cannot select", + MI); + return false; + } + LLVM_DEBUG(MIIMaintainer.reportFullyCreatedInstrs()); } - - // Dump the range of instructions that MI expanded into. - LLVM_DEBUG({ - auto InsertedBegin = ReachedBegin ? MBB->begin() : std::next(MII); - dbgs() << "Into:\n"; - for (auto &InsertedMI : make_range(InsertedBegin, AfterIt)) - dbgs() << " " << InsertedMI; - dbgs() << '\n'; - }); } } @@ -222,16 +231,10 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) { continue; } // Try to find redundant copies b/w vregs of the same register class. - bool ReachedBegin = false; - for (auto MII = std::prev(MBB.end()), Begin = MBB.begin(); !ReachedBegin;) { - // Select this instruction. + for (auto MII = MBB.rbegin(), End = MBB.rend(); MII != End;) { MachineInstr &MI = *MII; + ++MII; - // And have our iterator point to the next instruction, if there is one. - if (MII == Begin) - ReachedBegin = true; - else - --MII; if (MI.getOpcode() != TargetOpcode::COPY) continue; Register SrcReg = MI.getOperand(1).getReg(); @@ -336,3 +339,42 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) { // FIXME: Should we accurately track changes? return true; } + +bool InstructionSelect::selectInstr(MachineInstr &MI) { + MachineRegisterInfo &MRI = ISel->MF->getRegInfo(); + + // We could have folded this instruction away already, making it dead. + // If so, erase it. + if (isTriviallyDead(MI, MRI)) { + LLVM_DEBUG(dbgs() << "Is dead.\n"); + salvageDebugInfo(MRI, MI); + MI.eraseFromParent(); + return true; + } + + // Eliminate hints or G_CONSTANT_FOLD_BARRIER. + if (isPreISelGenericOptimizationHint(MI.getOpcode()) || + MI.getOpcode() == TargetOpcode::G_CONSTANT_FOLD_BARRIER) { + auto [DstReg, SrcReg] = MI.getFirst2Regs(); + + // At this point, the destination register class of the op may have + // been decided. + // + // Propagate that through to the source register. + const TargetRegisterClass *DstRC = MRI.getRegClassOrNull(DstReg); + if (DstRC) + MRI.setRegClass(SrcReg, DstRC); + assert(canReplaceReg(DstReg, SrcReg, MRI) && + "Must be able to replace dst with src!"); + MI.eraseFromParent(); + MRI.replaceRegWith(DstReg, SrcReg); + return true; + } + + if (MI.getOpcode() == TargetOpcode::G_INVOKE_REGION_START) { + MI.eraseFromParent(); + return true; + } + + return ISel->select(MI); +} diff --git a/llvm/unittests/CodeGen/GlobalISel/CMakeLists.txt b/llvm/unittests/CodeGen/GlobalISel/CMakeLists.txt index 253b90d5dc1f8..bce91c1ed6173 100644 --- a/llvm/unittests/CodeGen/GlobalISel/CMakeLists.txt +++ b/llvm/unittests/CodeGen/GlobalISel/CMakeLists.txt @@ -28,4 +28,5 @@ add_llvm_unittest(GlobalISelTests GISelUtilsTest.cpp GISelAliasTest.cpp CallLowering.cpp + InstructionSelectTest.cpp ) diff --git a/llvm/unittests/CodeGen/GlobalISel/InstructionSelectTest.cpp b/llvm/unittests/CodeGen/GlobalISel/InstructionSelectTest.cpp new file mode 100644 index 0000000000000..7fbccf7160e17 --- /dev/null +++ b/llvm/unittests/CodeGen/GlobalISel/InstructionSelectTest.cpp @@ -0,0 +1,76 @@ +#include "llvm/CodeGen/GlobalISel/InstructionSelect.h" +#include "GISelMITest.h" +#include "llvm/CodeGen/GlobalISel/InstructionSelector.h" +#include "llvm/CodeGen/TargetPassConfig.h" +#include "llvm/IR/LegacyPassManager.h" + +namespace { + +class EraseMockInstructionSelector : public InstructionSelector { +public: + int NumSelected = 0; + SmallVector MIs; + + bool select(MachineInstr &MI) override { + ++NumSelected; + switch (NumSelected) { + case 1: + EXPECT_EQ(&MI, MIs[8]); + // Erase previous instructions + MIs[7]->eraseFromParent(); + MIs[6]->eraseFromParent(); + // Don't erase this MI before step 3 to prevent DCE + return true; + case 2: + EXPECT_EQ(&MI, MIs[5]); + // Erase previous instructions reversed + MIs[3]->eraseFromParent(); + MIs[4]->eraseFromParent(); + MI.eraseFromParent(); + return true; + case 3: + EXPECT_EQ(&MI, MIs[2]); + MIs[8]->eraseFromParent(); + // Erase first instructions + MIs[0]->eraseFromParent(); + MIs[1]->eraseFromParent(); + MI.eraseFromParent(); + return true; + default: + ADD_FAILURE(); + return false; + } + } + + void setupGeneratedPerFunctionState(MachineFunction &MF) override {} +}; + +TEST_F(AArch64GISelMITest, TestInstructionSelectErase) { + StringRef MIRString = R"( + $x0 = COPY %2(s64) + $x0 = COPY %2(s64) + $x0 = COPY %2(s64) + $x0 = COPY %2(s64) + $x0 = COPY %2(s64) + $x0 = COPY %2(s64) +)"; + setUp(MIRString); + if (!TM) + GTEST_SKIP(); + + legacy::PassManager PM; + std::unique_ptr TPC(TM->createPassConfig(PM)); + + EraseMockInstructionSelector ISel; + ISel.TPC = TPC.get(); + for (auto &MI : *EntryMBB) { + ISel.MIs.push_back(&MI); + } + + InstructionSelect ISelPass; + ISelPass.setInstructionSelector(&ISel); + ISelPass.selectMachineFunction(*MF); + EXPECT_EQ(ISel.NumSelected, 3); +} + +} // namespace