From 9e7315912656628b606e884e39cdeb261b476f16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= Date: Fri, 20 Sep 2024 09:54:05 +0200 Subject: [PATCH] [NFC][EarlyIfConverter] Replace boolean Predicate for a class (#108519) Currently SSAIfConv is used in 2 scenarios. Generalize them to support more scenarios. --- llvm/lib/CodeGen/EarlyIfConversion.cpp | 258 ++++++++++++------------- 1 file changed, 121 insertions(+), 137 deletions(-) diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp index 99ffc01ca4ebb9..1b200c5d49189e 100644 --- a/llvm/lib/CodeGen/EarlyIfConversion.cpp +++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp @@ -122,6 +122,18 @@ class SSAIfConv { /// The branch condition determined by analyzeBranch. SmallVector Cond; + class PredicationStrategyBase { + public: + virtual bool canConvertIf(MachineBasicBlock *Tail) { return true; } + virtual bool canPredicateInstr(const MachineInstr &I) = 0; + virtual void predicateBlock(MachineBasicBlock *MBB, + ArrayRef Cond, + bool Reverse) = 0; + virtual ~PredicationStrategyBase() = default; + }; + + PredicationStrategyBase &Predicate; + private: /// Instructions in Head that define values used by the conditional blocks. /// The hoisted instructions must be inserted after these instructions. @@ -137,10 +149,6 @@ class SSAIfConv { /// and FBB. MachineBasicBlock::iterator InsertionPoint; - /// Return true if all non-terminator instructions in MBB can be safely - /// speculated. - bool canSpeculateInstrs(MachineBasicBlock *MBB); - /// Return true if all non-terminator instructions in MBB can be safely /// predicated. bool canPredicateInstrs(MachineBasicBlock *MBB); @@ -149,10 +157,6 @@ class SSAIfConv { /// Return false if any dependency is incompatible with if conversion. bool InstrDependenciesAllowIfConv(MachineInstr *I); - /// Predicate all instructions of the basic block with current condition - /// except for terminators. Reverse the condition if ReversePredicate is set. - void PredicateBlock(MachineBasicBlock *MBB, bool ReversePredicate); - /// Find a valid insertion point in Head. bool findInsertionPoint(); @@ -163,7 +167,8 @@ class SSAIfConv { void rewritePHIOperands(); public: - SSAIfConv(MachineFunction &MF) { + SSAIfConv(PredicationStrategyBase &Predicate, MachineFunction &MF) + : Predicate(Predicate) { TII = MF.getSubtarget().getInstrInfo(); TRI = MF.getSubtarget().getRegisterInfo(); MRI = &MF.getRegInfo(); @@ -175,77 +180,14 @@ class SSAIfConv { /// canConvertIf - If the sub-CFG headed by MBB can be if-converted, /// initialize the internal state, and return true. - /// If predicate is set try to predicate the block otherwise try to - /// speculatively execute it. - bool canConvertIf(MachineBasicBlock *MBB, bool Predicate = false); + bool canConvertIf(MachineBasicBlock *MBB); /// convertIf - If-convert the last block passed to canConvertIf(), assuming /// it is possible. Add any blocks that are to be erased to RemoveBlocks. - void convertIf(SmallVectorImpl &RemoveBlocks, - bool Predicate = false); + void convertIf(SmallVectorImpl &RemoveBlocks); }; } // end anonymous namespace - -/// canSpeculateInstrs - Returns true if all the instructions in MBB can safely -/// be speculated. The terminators are not considered. -/// -/// If instructions use any values that are defined in the head basic block, -/// the defining instructions are added to InsertAfter. -/// -/// Any clobbered regunits are added to ClobberedRegUnits. -/// -bool SSAIfConv::canSpeculateInstrs(MachineBasicBlock *MBB) { - // Reject any live-in physregs. It's probably CPSR/EFLAGS, and very hard to - // get right. - if (!MBB->livein_empty()) { - LLVM_DEBUG(dbgs() << printMBBReference(*MBB) << " has live-ins.\n"); - return false; - } - - unsigned InstrCount = 0; - - // Check all instructions, except the terminators. It is assumed that - // terminators never have side effects or define any used register values. - for (MachineInstr &MI : - llvm::make_range(MBB->begin(), MBB->getFirstTerminator())) { - if (MI.isDebugInstr()) - continue; - - if (++InstrCount > BlockInstrLimit && !Stress) { - LLVM_DEBUG(dbgs() << printMBBReference(*MBB) << " has more than " - << BlockInstrLimit << " instructions.\n"); - return false; - } - - // There shouldn't normally be any phis in a single-predecessor block. - if (MI.isPHI()) { - LLVM_DEBUG(dbgs() << "Can't hoist: " << MI); - return false; - } - - // Don't speculate loads. Note that it may be possible and desirable to - // speculate GOT or constant pool loads that are guaranteed not to trap, - // but we don't support that for now. - if (MI.mayLoad()) { - LLVM_DEBUG(dbgs() << "Won't speculate load: " << MI); - return false; - } - - // We never speculate stores, so an AA pointer isn't necessary. - bool DontMoveAcrossStore = true; - if (!MI.isSafeToMove(DontMoveAcrossStore)) { - LLVM_DEBUG(dbgs() << "Can't speculate: " << MI); - return false; - } - - // Check for any dependencies on Head instructions. - if (!InstrDependenciesAllowIfConv(&MI)) - return false; - } - return true; -} - /// Check that there is no dependencies preventing if conversion. /// /// If instruction uses any values that are defined in the head basic block, @@ -319,17 +261,8 @@ bool SSAIfConv::canPredicateInstrs(MachineBasicBlock *MBB) { return false; } - // Check that instruction is predicable - if (!TII->isPredicable(*I)) { - LLVM_DEBUG(dbgs() << "Isn't predicable: " << *I); - return false; - } - - // Check that instruction is not already predicated. - if (TII->isPredicated(*I) && !TII->canPredicatePredicatedInstr(*I)) { - LLVM_DEBUG(dbgs() << "Is already predicated: " << *I); + if (!Predicate.canPredicateInstr(*I)) return false; - } // Check for any dependencies on Head instructions. if (!InstrDependenciesAllowIfConv(&(*I))) @@ -338,24 +271,6 @@ bool SSAIfConv::canPredicateInstrs(MachineBasicBlock *MBB) { return true; } -// Apply predicate to all instructions in the machine block. -void SSAIfConv::PredicateBlock(MachineBasicBlock *MBB, bool ReversePredicate) { - auto Condition = Cond; - if (ReversePredicate) { - bool CanRevCond = !TII->reverseBranchCondition(Condition); - assert(CanRevCond && "Reversed predicate is not supported"); - (void)CanRevCond; - } - // Terminators don't need to be predicated as they will be removed. - for (MachineBasicBlock::iterator I = MBB->begin(), - E = MBB->getFirstTerminator(); - I != E; ++I) { - if (I->isDebugInstr()) - continue; - TII->PredicateInstruction(*I, Condition); - } -} - /// Find an insertion point in Head for the speculated instructions. The /// insertion point must be: /// @@ -434,7 +349,7 @@ bool SSAIfConv::findInsertionPoint() { /// canConvertIf - analyze the sub-cfg rooted in MBB, and return true if it is /// a potential candidate for if-conversion. Fill out the internal state. /// -bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) { +bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) { Head = MBB; TBB = FBB = Tail = nullptr; @@ -474,14 +389,6 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) { << printMBBReference(*Tail) << '\n'); } - // This is a triangle or a diamond. - // Skip if we cannot predicate and there are no phis skip as there must be - // side effects that can only be handled with predication. - if (!Predicate && (Tail->empty() || !Tail->front().isPHI())) { - LLVM_DEBUG(dbgs() << "No phis in tail.\n"); - return false; - } - // The branch we're looking to eliminate must be analyzable. Cond.clear(); if (TII->analyzeBranch(*Head, TBB, FBB, Cond)) { @@ -489,6 +396,10 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) { return false; } + if (!Predicate.canConvertIf(Tail)) { + return false; + } + // This is weird, probably some sort of degenerate CFG. if (!TBB) { LLVM_DEBUG(dbgs() << "analyzeBranch didn't find conditional branch.\n"); @@ -536,17 +447,9 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) { // Check that the conditional instructions can be speculated. InsertAfter.clear(); ClobberedRegUnits.reset(); - if (Predicate) { - if (TBB != Tail && !canPredicateInstrs(TBB)) + for (MachineBasicBlock *MBB : {TBB, FBB}) + if (MBB != Tail && !canPredicateInstrs(MBB)) return false; - if (FBB != Tail && !canPredicateInstrs(FBB)) - return false; - } else { - if (TBB != Tail && !canSpeculateInstrs(TBB)) - return false; - if (FBB != Tail && !canSpeculateInstrs(FBB)) - return false; - } // Try to find a valid insertion point for the speculated instructions in the // head basic block. @@ -679,8 +582,7 @@ void SSAIfConv::rewritePHIOperands() { /// /// Any basic blocks that need to be erased will be added to RemoveBlocks. /// -void SSAIfConv::convertIf(SmallVectorImpl &RemoveBlocks, - bool Predicate) { +void SSAIfConv::convertIf(SmallVectorImpl &RemoveBlocks) { assert(Head && Tail && TBB && FBB && "Call canConvertIf first."); // Update statistics. @@ -690,16 +592,15 @@ void SSAIfConv::convertIf(SmallVectorImpl &RemoveBlocks, ++NumDiamondsConv; // Move all instructions into Head, except for the terminators. - if (TBB != Tail) { - if (Predicate) - PredicateBlock(TBB, /*ReversePredicate=*/false); - Head->splice(InsertionPoint, TBB, TBB->begin(), TBB->getFirstTerminator()); - } - if (FBB != Tail) { - if (Predicate) - PredicateBlock(FBB, /*ReversePredicate=*/true); - Head->splice(InsertionPoint, FBB, FBB->begin(), FBB->getFirstTerminator()); + for (MachineBasicBlock *MBB : {TBB, FBB}) { + if (MBB != Tail) { + // reverse the condition for the false bb + Predicate.predicateBlock(MBB, Cond, MBB == FBB); + Head->splice(InsertionPoint, MBB, MBB->begin(), + MBB->getFirstTerminator()); + } } + // Are there extra Tail predecessors? bool ExtraPreds = Tail->pred_size() != 2; if (ExtraPreds) @@ -863,6 +764,46 @@ template Remark &operator<<(Remark &R, Cycles C) { } } // anonymous namespace +class SpeculateStrategy : public SSAIfConv::PredicationStrategyBase { +public: + bool canConvertIf(MachineBasicBlock *Tail) override { + // This is a triangle or a diamond. + // Skip if we cannot predicate and there are no phis skip as there must + // be side effects that can only be handled with predication. + if (Tail->empty() || !Tail->front().isPHI()) { + LLVM_DEBUG(dbgs() << "No phis in tail.\n"); + return false; + } + return true; + } + + bool canPredicateInstr(const MachineInstr &I) override { + // Don't speculate loads. Note that it may be possible and desirable to + // speculate GOT or constant pool loads that are guaranteed not to trap, + // but we don't support that for now. + if (I.mayLoad()) { + LLVM_DEBUG(dbgs() << "Won't speculate load: " << I); + return false; + } + + // We never speculate stores, so an AA pointer isn't necessary. + bool DontMoveAcrossStore = true; + if (!I.isSafeToMove(DontMoveAcrossStore)) { + LLVM_DEBUG(dbgs() << "Can't speculate: " << I); + return false; + } + return true; + } + + void predicateBlock(MachineBasicBlock *MBB, ArrayRef Cond, + bool Reverse) + override { /* do nothing, everything is speculatable and it's valid to + move the instructions into the head */ + } + + ~SpeculateStrategy() override = default; +}; + /// Apply cost model and heuristics to the if-conversion in IfConv. /// Return true if the conversion is a good idea. /// @@ -1096,7 +1037,8 @@ bool EarlyIfConverter::runOnMachineFunction(MachineFunction &MF) { MinInstr = nullptr; bool Changed = false; - SSAIfConv IfConv(MF); + SpeculateStrategy Speculate; + SSAIfConv IfConv(Speculate, MF); // Visit blocks in dominator tree post-order. The post-order enables nested // if-conversion in a single pass. The tryConvertIf() function may erase @@ -1158,6 +1100,48 @@ void EarlyIfPredicator::getAnalysisUsage(AnalysisUsage &AU) const { MachineFunctionPass::getAnalysisUsage(AU); } +class PredicatorStrategy : public SSAIfConv::PredicationStrategyBase { + const TargetInstrInfo *TII = nullptr; + +public: + PredicatorStrategy(const TargetInstrInfo *TII) : TII(TII) {} + + bool canPredicateInstr(const MachineInstr &I) override { + // Check that instruction is predicable + if (!TII->isPredicable(I)) { + LLVM_DEBUG(dbgs() << "Isn't predicable: " << I); + return false; + } + + // Check that instruction is not already predicated. + if (TII->isPredicated(I) && !TII->canPredicatePredicatedInstr(I)) { + LLVM_DEBUG(dbgs() << "Is already predicated: " << I); + return false; + } + return true; + } + + void predicateBlock(MachineBasicBlock *MBB, ArrayRef Cond, + bool Reverse) override { + SmallVector Condition(Cond.begin(), Cond.end()); + if (Reverse) { + bool CanRevCond = !TII->reverseBranchCondition(Condition); + assert(CanRevCond && "Reversed predicate is not supported"); + (void)CanRevCond; + } + // Terminators don't need to be predicated as they will be removed. + for (MachineBasicBlock::iterator I = MBB->begin(), + E = MBB->getFirstTerminator(); + I != E; ++I) { + if (I->isDebugInstr()) + continue; + TII->PredicateInstruction(*I, Condition); + } + } + + ~PredicatorStrategy() override = default; +}; + /// Apply the target heuristic to decide if the transformation is profitable. bool EarlyIfPredicator::shouldConvertIf(SSAIfConv &IfConv) { auto TrueProbability = MBPI->getEdgeProbability(IfConv.Head, IfConv.TBB); @@ -1202,11 +1186,10 @@ bool EarlyIfPredicator::shouldConvertIf(SSAIfConv &IfConv) { bool EarlyIfPredicator::tryConvertIf(SSAIfConv &IfConv, MachineBasicBlock *MBB) { bool Changed = false; - while (IfConv.canConvertIf(MBB, /*Predicate=*/true) && - shouldConvertIf(IfConv)) { + while (IfConv.canConvertIf(MBB) && shouldConvertIf(IfConv)) { // If-convert MBB and update analyses. SmallVector RemoveBlocks; - IfConv.convertIf(RemoveBlocks, /*Predicate=*/true); + IfConv.convertIf(RemoveBlocks); Changed = true; updateDomTree(DomTree, IfConv, RemoveBlocks); for (MachineBasicBlock *MBB : RemoveBlocks) @@ -1232,7 +1215,8 @@ bool EarlyIfPredicator::runOnMachineFunction(MachineFunction &MF) { MBPI = &getAnalysis().getMBPI(); bool Changed = false; - SSAIfConv IfConv(MF); + PredicatorStrategy Predicate(TII); + SSAIfConv IfConv(Predicate, MF); // Visit blocks in dominator tree post-order. The post-order enables nested // if-conversion in a single pass. The tryConvertIf() function may erase