[EarlyIfCvt] Take branch probablities into consideration #97808
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.6-beta.1
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-backend-aarch64 Author: Pengcheng Wang (wangpc-pp) ChangesFor predictable branches, we shouldn't convert them to selects. For existing targets that enabled early if-conversion, we override Full diff: https://github.com/llvm/llvm-project/pull/97808.diff 8 Files Affected:
diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp
index 5f3e85077cb56..fc94879c90829 100644
--- a/llvm/lib/CodeGen/EarlyIfConversion.cpp
+++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp
@@ -180,6 +180,9 @@ class SSAIfConv {
/// speculatively execute it.
bool canConvertIf(MachineBasicBlock *MBB, bool Predicate = false);
+ bool isProfitable(const MachineBranchProbabilityInfo *MBPI,
+ TargetSchedModel SchedModel);
+
/// convertIf - If-convert the last block passed to canConvertIf(), assuming
/// it is possible. Add any erased blocks to RemovedBlocks.
void convertIf(SmallVectorImpl<MachineBasicBlock *> &RemovedBlocks,
@@ -561,6 +564,45 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) {
return true;
}
+/// Apply the target heuristic to decide if the transformation is profitable.
+bool SSAIfConv::isProfitable(const MachineBranchProbabilityInfo *MBPI,
+ TargetSchedModel SchedModel) {
+ auto TrueProbability = MBPI->getEdgeProbability(Head, TBB);
+ if (isTriangle()) {
+ MachineBasicBlock &IfBlock = (TBB == Tail) ? *FBB : *TBB;
+
+ unsigned ExtraPredCost = 0;
+ unsigned Cycles = 0;
+ for (MachineInstr &I : IfBlock) {
+ unsigned NumCycles = SchedModel.computeInstrLatency(&I, false);
+ if (NumCycles > 1)
+ Cycles += NumCycles - 1;
+ ExtraPredCost += TII->getPredicationCost(I);
+ }
+
+ return TII->isProfitableToIfCvt(IfBlock, Cycles, ExtraPredCost,
+ TrueProbability);
+ }
+ unsigned TExtra = 0;
+ unsigned FExtra = 0;
+ unsigned TCycle = 0;
+ unsigned FCycle = 0;
+ for (MachineInstr &I : *TBB) {
+ unsigned NumCycles = SchedModel.computeInstrLatency(&I, false);
+ if (NumCycles > 1)
+ TCycle += NumCycles - 1;
+ TExtra += TII->getPredicationCost(I);
+ }
+ for (MachineInstr &I : *FBB) {
+ unsigned NumCycles = SchedModel.computeInstrLatency(&I, false);
+ if (NumCycles > 1)
+ FCycle += NumCycles - 1;
+ FExtra += TII->getPredicationCost(I);
+ }
+ return TII->isProfitableToIfCvt(*TBB, TCycle, TExtra, *FBB, FCycle, FExtra,
+ TrueProbability);
+}
+
/// \return true iff the two registers are known to have the same value.
static bool hasSameValue(const MachineRegisterInfo &MRI,
const TargetInstrInfo *TII, Register TReg,
@@ -760,9 +802,11 @@ namespace {
class EarlyIfConverter : public MachineFunctionPass {
const TargetInstrInfo *TII = nullptr;
const TargetRegisterInfo *TRI = nullptr;
- MCSchedModel SchedModel;
+ MCSchedModel MCSchedModel;
+ TargetSchedModel TargetSchedModel;
MachineRegisterInfo *MRI = nullptr;
MachineDominatorTree *DomTree = nullptr;
+ MachineBranchProbabilityInfo *MBPI = nullptr;
MachineLoopInfo *Loops = nullptr;
MachineTraceMetrics *Traces = nullptr;
MachineTraceMetrics::Ensemble *MinInstr = nullptr;
@@ -871,6 +915,9 @@ bool EarlyIfConverter::shouldConvertIf() {
// Do not try to if-convert if the condition has a high chance of being
// predictable.
+ if (!IfConv.isProfitable(MBPI, TargetSchedModel))
+ return false;
+
MachineLoop *CurrentLoop = Loops->getLoopFor(IfConv.Head);
// If the condition is in a loop, consider it predictable if the condition
// itself or all its operands are loop-invariant. E.g. this considers a load
@@ -911,7 +958,7 @@ bool EarlyIfConverter::shouldConvertIf() {
FBBTrace.getCriticalPath());
// Set a somewhat arbitrary limit on the critical path extension we accept.
- unsigned CritLimit = SchedModel.MispredictPenalty/2;
+ unsigned CritLimit = MCSchedModel.MispredictPenalty / 2;
MachineBasicBlock &MBB = *IfConv.Head;
MachineOptimizationRemarkEmitter MORE(*MBB.getParent(), nullptr);
@@ -1084,9 +1131,11 @@ bool EarlyIfConverter::runOnMachineFunction(MachineFunction &MF) {
TII = STI.getInstrInfo();
TRI = STI.getRegisterInfo();
- SchedModel = STI.getSchedModel();
+ MCSchedModel = STI.getSchedModel();
+ TargetSchedModel.init(&STI);
MRI = &MF.getRegInfo();
DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
+ MBPI = &getAnalysis<MachineBranchProbabilityInfoWrapperPass>().getMBPI();
Loops = &getAnalysis<MachineLoopInfo>();
Traces = &getAnalysis<MachineTraceMetrics>();
MinInstr = nullptr;
@@ -1155,43 +1204,8 @@ void EarlyIfPredicator::getAnalysisUsage(AnalysisUsage &AU) const {
MachineFunctionPass::getAnalysisUsage(AU);
}
-/// Apply the target heuristic to decide if the transformation is profitable.
bool EarlyIfPredicator::shouldConvertIf() {
- auto TrueProbability = MBPI->getEdgeProbability(IfConv.Head, IfConv.TBB);
- if (IfConv.isTriangle()) {
- MachineBasicBlock &IfBlock =
- (IfConv.TBB == IfConv.Tail) ? *IfConv.FBB : *IfConv.TBB;
-
- unsigned ExtraPredCost = 0;
- unsigned Cycles = 0;
- for (MachineInstr &I : IfBlock) {
- unsigned NumCycles = SchedModel.computeInstrLatency(&I, false);
- if (NumCycles > 1)
- Cycles += NumCycles - 1;
- ExtraPredCost += TII->getPredicationCost(I);
- }
-
- return TII->isProfitableToIfCvt(IfBlock, Cycles, ExtraPredCost,
- TrueProbability);
- }
- unsigned TExtra = 0;
- unsigned FExtra = 0;
- unsigned TCycle = 0;
- unsigned FCycle = 0;
- for (MachineInstr &I : *IfConv.TBB) {
- unsigned NumCycles = SchedModel.computeInstrLatency(&I, false);
- if (NumCycles > 1)
- TCycle += NumCycles - 1;
- TExtra += TII->getPredicationCost(I);
- }
- for (MachineInstr &I : *IfConv.FBB) {
- unsigned NumCycles = SchedModel.computeInstrLatency(&I, false);
- if (NumCycles > 1)
- FCycle += NumCycles - 1;
- FExtra += TII->getPredicationCost(I);
- }
- return TII->isProfitableToIfCvt(*IfConv.TBB, TCycle, TExtra, *IfConv.FBB,
- FCycle, FExtra, TrueProbability);
+ return IfConv.isProfitable(MBPI, SchedModel);
}
/// Attempt repeated if-conversion on MBB, return true if successful.
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 93278a2ba0d09..616ef0310ddd4 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -699,6 +699,19 @@ static unsigned canFoldIntoCSel(const MachineRegisterInfo &MRI, unsigned VReg,
return Opc;
}
+bool AArch64InstrInfo::isProfitableToIfCvt(
+ MachineBasicBlock &MBB, unsigned NumCycles, unsigned ExtraPredCycles,
+ BranchProbability Probability) const {
+ return MBB.getParent()->getSubtarget().enableEarlyIfConversion();
+}
+
+bool AArch64InstrInfo::isProfitableToIfCvt(
+ MachineBasicBlock &TMBB, unsigned NumTCycles, unsigned ExtraTCycles,
+ MachineBasicBlock &FMBB, unsigned NumFCycles, unsigned ExtraFCycles,
+ BranchProbability Probability) const {
+ return TMBB.getParent()->getSubtarget().enableEarlyIfConversion();
+}
+
bool AArch64InstrInfo::canInsertSelect(const MachineBasicBlock &MBB,
ArrayRef<MachineOperand> Cond,
Register DstReg, Register TrueReg,
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index 792e0c3063b10..476cb48bc21a7 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -393,6 +393,13 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
bool
reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;
+ bool isProfitableToIfCvt(MachineBasicBlock &MBB, unsigned NumCycles,
+ unsigned ExtraPredCycles,
+ BranchProbability Probability) const override;
+ bool isProfitableToIfCvt(MachineBasicBlock &TMBB, unsigned NumTCycles,
+ unsigned ExtraTCycles, MachineBasicBlock &FMBB,
+ unsigned NumFCycles, unsigned ExtraFCycles,
+ BranchProbability Probability) const override;
bool canInsertSelect(const MachineBasicBlock &, ArrayRef<MachineOperand> Cond,
Register, Register, Register, int &, int &,
int &) const override;
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index cc1b9ac0c9ecd..43a1b7831d9ec 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -3219,6 +3219,20 @@ bool SIInstrInfo::reverseBranchCondition(
return true;
}
+bool SIInstrInfo::isProfitableToIfCvt(MachineBasicBlock &MBB,
+ unsigned NumCycles,
+ unsigned ExtraPredCycles,
+ BranchProbability Probability) const {
+ return MBB.getParent()->getSubtarget().enableEarlyIfConversion();
+}
+
+bool SIInstrInfo::isProfitableToIfCvt(
+ MachineBasicBlock &TMBB, unsigned NumTCycles, unsigned ExtraTCycles,
+ MachineBasicBlock &FMBB, unsigned NumFCycles, unsigned ExtraFCycles,
+ BranchProbability Probability) const {
+ return TMBB.getParent()->getSubtarget().enableEarlyIfConversion();
+}
+
bool SIInstrInfo::canInsertSelect(const MachineBasicBlock &MBB,
ArrayRef<MachineOperand> Cond,
Register DstReg, Register TrueReg,
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 84bb73cc9a796..37e713c16646d 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -362,6 +362,13 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
bool reverseBranchCondition(
SmallVectorImpl<MachineOperand> &Cond) const override;
+ bool isProfitableToIfCvt(MachineBasicBlock &MBB, unsigned NumCycles,
+ unsigned ExtraPredCycles,
+ BranchProbability Probability) const override;
+ bool isProfitableToIfCvt(MachineBasicBlock &TMBB, unsigned NumTCycles,
+ unsigned ExtraTCycles, MachineBasicBlock &FMBB,
+ unsigned NumFCycles, unsigned ExtraFCycles,
+ BranchProbability Probability) const override;
bool canInsertSelect(const MachineBasicBlock &MBB,
ArrayRef<MachineOperand> Cond, Register DstReg,
Register TrueReg, Register FalseReg, int &CondCycles,
diff --git a/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp b/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
index 16bbfd44ef8a9..af735fae8ef4f 100644
--- a/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
@@ -766,10 +766,9 @@ bool SystemZInstrInfo::isPredicable(const MachineInstr &MI) const {
return false;
}
-bool SystemZInstrInfo::
-isProfitableToIfCvt(MachineBasicBlock &MBB,
- unsigned NumCycles, unsigned ExtraPredCycles,
- BranchProbability Probability) const {
+bool SystemZInstrInfo::isProfitableToIfCvt(
+ MachineBasicBlock &MBB, unsigned NumCycles, unsigned ExtraPredCycles,
+ BranchProbability Probability) const {
// Avoid using conditional returns at the end of a loop (since then
// we'd need to emit an unconditional branch to the beginning anyway,
// making the loop body longer). This doesn't apply for low-probability
@@ -778,8 +777,10 @@ isProfitableToIfCvt(MachineBasicBlock &MBB,
// However, since Compare and Trap instructions cost the same as a regular
// Compare instruction, we should allow the if conversion to convert this
// into a Conditional Compare regardless of the branch probability.
- if (MBB.getLastNonDebugInstr()->getOpcode() != SystemZ::Trap &&
- MBB.succ_empty() && Probability < BranchProbability(1, 8))
+ if (auto MI = MBB.getLastNonDebugInstr();
+ MI == MBB.end() ||
+ (MI->getOpcode() != SystemZ::Trap && MBB.succ_empty() &&
+ Probability < BranchProbability(1, 8)))
return false;
// For now only convert single instructions.
return NumCycles == 1;
@@ -791,8 +792,7 @@ isProfitableToIfCvt(MachineBasicBlock &TMBB,
MachineBasicBlock &FMBB,
unsigned NumCyclesF, unsigned ExtraPredCyclesF,
BranchProbability Probability) const {
- // For now avoid converting mutually-exclusive cases.
- return false;
+ return TMBB.getParent()->getSubtarget().enableEarlyIfConversion();
}
bool SystemZInstrInfo::
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index fab7c167e385f..0974d755c96b3 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -4094,6 +4094,20 @@ unsigned X86InstrInfo::insertBranch(MachineBasicBlock &MBB,
return Count;
}
+bool X86InstrInfo::isProfitableToIfCvt(MachineBasicBlock &MBB,
+ unsigned NumCycles,
+ unsigned ExtraPredCycles,
+ BranchProbability Probability) const {
+ return MBB.getParent()->getSubtarget().enableEarlyIfConversion();
+}
+
+bool X86InstrInfo::isProfitableToIfCvt(
+ MachineBasicBlock &TMBB, unsigned NumTCycles, unsigned ExtraTCycles,
+ MachineBasicBlock &FMBB, unsigned NumFCycles, unsigned ExtraFCycles,
+ BranchProbability Probability) const {
+ return TMBB.getParent()->getSubtarget().enableEarlyIfConversion();
+}
+
bool X86InstrInfo::canInsertSelect(const MachineBasicBlock &MBB,
ArrayRef<MachineOperand> Cond,
Register DstReg, Register TrueReg,
diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h
index eaa3dd0893948..b970edbc005f8 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.h
+++ b/llvm/lib/Target/X86/X86InstrInfo.h
@@ -409,6 +409,13 @@ class X86InstrInfo final : public X86GenInstrInfo {
MachineBasicBlock *FBB, ArrayRef<MachineOperand> Cond,
const DebugLoc &DL,
int *BytesAdded = nullptr) const override;
+ bool isProfitableToIfCvt(MachineBasicBlock &MBB, unsigned NumCycles,
+ unsigned ExtraPredCycles,
+ BranchProbability Probability) const override;
+ bool isProfitableToIfCvt(MachineBasicBlock &TMBB, unsigned NumTCycles,
+ unsigned ExtraTCycles, MachineBasicBlock &FMBB,
+ unsigned NumFCycles, unsigned ExtraFCycles,
+ BranchProbability Probability) const override;
bool canInsertSelect(const MachineBasicBlock &, ArrayRef<MachineOperand> Cond,
Register, Register, Register, int &, int &,
int &) const override;
|
fhahn
reviewed
Jul 8, 2024
wangpc-pp
changed the base branch from
main
to
users/wangpc-pp/spr/main.earlyifcvt-take-branch-probablities-into-consideration
mgudim
reviewed
Jul 10, 2024
| /// Return true if the target will always try to convert predictable branches | ||
| /// to selects. | ||
| virtual bool shouldConvertPredictableBranches() const { return true; } | ||
|
|
There was a problem hiding this comment.
The default should be false
There was a problem hiding this comment.
I think the current behavior is we will convert predictable branches. To not touch too much targets, I leave it to be true here.
It looks like this MR is only adding a target hook, so this title doesn't make sense to me |
I was planning to add support to RISCV target, but it depends on your early if-conversion patch. I will hold this PR until RISC-V has got its early if-conversion support. |
|
Needs target + test? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For predictable branches, we shouldn't convert them to selects.
We add a hook
shouldConvertPredictableBranchesto let the targetdecides if we should convert predictable branches.