[llvm] [NFC][EarlyIfConverter] Replace boolean Preadicate for a class (PR #108519)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 13 01:59:42 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86
Author: Juan Manuel Martinez Caamaño (jmmartinez)
<details>
<summary>Changes</summary>
Currently SSAIfConv is used in 2 scenarios. Generalize them to support more scenarios.
---
Full diff: https://github.com/llvm/llvm-project/pull/108519.diff
1 Files Affected:
- (modified) llvm/lib/CodeGen/EarlyIfConversion.cpp (+117-137)
``````````diff
diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp
index 99ffc01ca4ebb9..ae3b367b09308a 100644
--- a/llvm/lib/CodeGen/EarlyIfConversion.cpp
+++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp
@@ -122,6 +122,17 @@ class SSAIfConv {
/// The branch condition determined by analyzeBranch.
SmallVector<MachineOperand, 4> Cond;
+ struct PredicationStrategyBase {
+ virtual bool canConvertIf(MachineBasicBlock *Tail) { return true; }
+ virtual bool canPredicateInstr(const MachineInstr &I) = 0;
+ virtual void predicateBlock(MachineBasicBlock *MBB,
+ ArrayRef<MachineOperand> 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 +148,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 +156,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 +166,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 +179,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<MachineBasicBlock *> &RemoveBlocks,
- bool Predicate = false);
+ void convertIf(SmallVectorImpl<MachineBasicBlock *> &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 +260,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 +270,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 +348,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 +388,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 +395,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 +446,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))
- return false;
- if (FBB != Tail && !canPredicateInstrs(FBB))
- return false;
- } else {
- if (TBB != Tail && !canSpeculateInstrs(TBB))
- return false;
- if (FBB != Tail && !canSpeculateInstrs(FBB))
+ for (MachineBasicBlock *MBB : {TBB, FBB})
+ if (MBB != Tail && !canPredicateInstrs(MBB))
return false;
- }
// Try to find a valid insertion point for the speculated instructions in the
// head basic block.
@@ -679,8 +581,7 @@ void SSAIfConv::rewritePHIOperands() {
///
/// Any basic blocks that need to be erased will be added to RemoveBlocks.
///
-void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
- bool Predicate) {
+void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks) {
assert(Head && Tail && TBB && FBB && "Call canConvertIf first.");
// Update statistics.
@@ -690,16 +591,15 @@ void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &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 +763,45 @@ template <typename Remark> Remark &operator<<(Remark &R, Cycles C) {
}
} // anonymous namespace
+struct SpeculateStrategy : SSAIfConv::PredicationStrategyBase {
+ 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<MachineOperand> 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 +1035,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 +1098,46 @@ void EarlyIfPredicator::getAnalysisUsage(AnalysisUsage &AU) const {
MachineFunctionPass::getAnalysisUsage(AU);
}
+struct PredicatorStrategy : SSAIfConv::PredicationStrategyBase {
+ const TargetInstrInfo *TII = nullptr;
+ 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<MachineOperand> Cond,
+ bool Reverse) override {
+ SmallVector<MachineOperand> 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 +1182,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<MachineBasicBlock *, 4> RemoveBlocks;
- IfConv.convertIf(RemoveBlocks, /*Predicate=*/true);
+ IfConv.convertIf(RemoveBlocks);
Changed = true;
updateDomTree(DomTree, IfConv, RemoveBlocks);
for (MachineBasicBlock *MBB : RemoveBlocks)
@@ -1232,7 +1211,8 @@ bool EarlyIfPredicator::runOnMachineFunction(MachineFunction &MF) {
MBPI = &getAnalysis<MachineBranchProbabilityInfoWrapperPass>().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
``````````
</details>
https://github.com/llvm/llvm-project/pull/108519
More information about the llvm-commits
mailing list