[llvm] d5ec01b - Revert "[NFC][EarlyIfConverter] Replace boolean Predicate for a class (#108519)" (#111372)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 7 07:03:15 PDT 2024
Author: Juan Manuel Martinez Caamaño
Date: 2024-10-07T16:03:11+02:00
New Revision: d5ec01b0dd20d8bdacc2810539686374e7ad5918
URL: https://github.com/llvm/llvm-project/commit/d5ec01b0dd20d8bdacc2810539686374e7ad5918
DIFF: https://github.com/llvm/llvm-project/commit/d5ec01b0dd20d8bdacc2810539686374e7ad5918.diff
LOG: Revert "[NFC][EarlyIfConverter] Replace boolean Predicate for a class (#108519)" (#111372)
This reverts commit 9e7315912656628b606e884e39cdeb261b476f16.
Added:
Modified:
llvm/lib/CodeGen/EarlyIfConversion.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp
index 2b97fa449f08f7..c827d5bdcf55ba 100644
--- a/llvm/lib/CodeGen/EarlyIfConversion.cpp
+++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp
@@ -122,18 +122,6 @@ class SSAIfConv {
/// The branch condition determined by analyzeBranch.
SmallVector<MachineOperand, 4> Cond;
- class PredicationStrategyBase {
- public:
- 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.
@@ -149,6 +137,10 @@ 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);
@@ -157,6 +149,10 @@ 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();
@@ -167,8 +163,7 @@ class SSAIfConv {
void rewritePHIOperands();
public:
- SSAIfConv(PredicationStrategyBase &Predicate, MachineFunction &MF)
- : Predicate(Predicate) {
+ SSAIfConv(MachineFunction &MF) {
TII = MF.getSubtarget().getInstrInfo();
TRI = MF.getSubtarget().getRegisterInfo();
MRI = &MF.getRegInfo();
@@ -180,14 +175,76 @@ class SSAIfConv {
/// canConvertIf - If the sub-CFG headed by MBB can be if-converted,
/// initialize the internal state, and return true.
- bool canConvertIf(MachineBasicBlock *MBB);
+ /// If predicate is set try to predicate the block otherwise try to
+ /// speculatively execute it.
+ bool canConvertIf(MachineBasicBlock *MBB, bool Predicate = false);
/// 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);
+ void convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
+ bool Predicate = false);
};
} // 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,
@@ -261,8 +318,17 @@ bool SSAIfConv::canPredicateInstrs(MachineBasicBlock *MBB) {
return false;
}
- if (!Predicate.canPredicateInstr(*I))
+ // 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;
+ }
// Check for any dependencies on Head instructions.
if (!InstrDependenciesAllowIfConv(&(*I)))
@@ -271,6 +337,24 @@ 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:
///
@@ -349,7 +433,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 SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) {
Head = MBB;
TBB = FBB = Tail = nullptr;
@@ -389,6 +473,14 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) {
<< 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)) {
@@ -396,10 +488,6 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) {
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");
@@ -447,9 +535,17 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) {
// Check that the conditional instructions can be speculated.
InsertAfter.clear();
ClobberedRegUnits.reset();
- for (MachineBasicBlock *MBB : {TBB, FBB})
- if (MBB != Tail && !canPredicateInstrs(MBB))
+ 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))
+ return false;
+ }
// Try to find a valid insertion point for the speculated instructions in the
// head basic block.
@@ -582,7 +678,8 @@ void SSAIfConv::rewritePHIOperands() {
///
/// Any basic blocks that need to be erased will be added to RemoveBlocks.
///
-void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks) {
+void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
+ bool Predicate) {
assert(Head && Tail && TBB && FBB && "Call canConvertIf first.");
// Update statistics.
@@ -592,15 +689,16 @@ void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks) {
++NumDiamondsConv;
// Move all instructions into Head, except for the terminators.
- 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());
- }
+ 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());
}
-
// Are there extra Tail predecessors?
bool ExtraPreds = Tail->pred_size() != 2;
if (ExtraPreds)
@@ -764,46 +862,6 @@ template <typename Remark> 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<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.
///
@@ -1037,8 +1095,7 @@ bool EarlyIfConverter::runOnMachineFunction(MachineFunction &MF) {
MinInstr = nullptr;
bool Changed = false;
- SpeculateStrategy Speculate;
- SSAIfConv IfConv(Speculate, MF);
+ SSAIfConv IfConv(MF);
// Visit blocks in dominator tree post-order. The post-order enables nested
// if-conversion in a single pass. The tryConvertIf() function may erase
@@ -1100,48 +1157,6 @@ 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<MachineOperand> Cond,
- bool Reverse) override {
- SmallVector<MachineOperand> Condition(Cond);
- 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);
@@ -1186,10 +1201,11 @@ bool EarlyIfPredicator::shouldConvertIf(SSAIfConv &IfConv) {
bool EarlyIfPredicator::tryConvertIf(SSAIfConv &IfConv,
MachineBasicBlock *MBB) {
bool Changed = false;
- while (IfConv.canConvertIf(MBB) && shouldConvertIf(IfConv)) {
+ while (IfConv.canConvertIf(MBB, /*Predicate=*/true) &&
+ shouldConvertIf(IfConv)) {
// If-convert MBB and update analyses.
SmallVector<MachineBasicBlock *, 4> RemoveBlocks;
- IfConv.convertIf(RemoveBlocks);
+ IfConv.convertIf(RemoveBlocks, /*Predicate=*/true);
Changed = true;
updateDomTree(DomTree, IfConv, RemoveBlocks);
for (MachineBasicBlock *MBB : RemoveBlocks)
@@ -1215,8 +1231,7 @@ bool EarlyIfPredicator::runOnMachineFunction(MachineFunction &MF) {
MBPI = &getAnalysis<MachineBranchProbabilityInfoWrapperPass>().getMBPI();
bool Changed = false;
- PredicatorStrategy Predicate(TII);
- SSAIfConv IfConv(Predicate, MF);
+ SSAIfConv IfConv(MF);
// Visit blocks in dominator tree post-order. The post-order enables nested
// if-conversion in a single pass. The tryConvertIf() function may erase
More information about the llvm-commits
mailing list