[llvm] [X86] Don't always separate conditions in `(br (and/or cond0, cond1))` into separate branches (PR #81689)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 2 23:19:22 PST 2024
================
@@ -2446,6 +2448,147 @@ SelectionDAGBuilder::EmitBranchForMergedCondition(const Value *Cond,
SL->SwitchCases.push_back(CB);
}
+// Collect dependencies on V recursively. This is used for the cost analysis in
+// `shouldKeepJumpConditionsTogether`.
+static bool
+collectInstructionDeps(SmallPtrSet<const Instruction *, 8> *Deps,
+ const Value *V,
+ SmallPtrSet<const Instruction *, 8> *Necessary = nullptr,
+ unsigned Depth = 0) {
+ // Return false if we have an incomplete count.
+ if (Depth >= SelectionDAG::MaxRecursionDepth)
+ return false;
+
+ auto *I = dyn_cast<Instruction>(V);
+ if (I == nullptr)
+ return true;
+
+ if (Necessary != nullptr) {
+ // This instruction is necessary for the other side of the condition so
+ // don't count it.
+ if (Necessary->contains(I))
+ return true;
+ }
+
+ // Already added this dep.
+ if (!Deps->insert(I).second)
+ return true;
+
+ for (unsigned OpIdx = 0, E = I->getNumOperands(); OpIdx < E; ++OpIdx)
+ if (!collectInstructionDeps(Deps, I->getOperand(OpIdx), Necessary,
+ Depth + 1))
+ return false;
+ return true;
+}
+
+bool SelectionDAGBuilder::shouldKeepJumpConditionsTogether(
+ const FunctionLoweringInfo &FuncInfo, const BranchInst &I,
+ Instruction::BinaryOps Opc, const Value *Lhs, const Value *Rhs,
+ TargetLoweringBase::CondMergingParams Params) const {
+ if (I.getNumSuccessors() != 2)
+ return false;
+
+ if (Params.BaseCost < 0)
+ return false;
+
+ // Baseline cost.
+ InstructionCost CostThresh = Params.BaseCost;
+
+ BranchProbabilityInfo *BPI = nullptr;
+ if (Params.LikelyBias || Params.UnlikelyBias)
+ BPI = FuncInfo.BPI;
+ if (BPI != nullptr) {
+ // See if we are either likely to get an early out or compute both lhs/rhs
+ // of the condition.
+ BasicBlock *IfFalse = I.getSuccessor(0);
+ BasicBlock *IfTrue = I.getSuccessor(1);
+
+ std::optional<bool> Likely;
+ if (BPI->isEdgeHot(I.getParent(), IfTrue))
+ Likely = true;
+ else if (BPI->isEdgeHot(I.getParent(), IfFalse))
+ Likely = false;
+
+ if (Likely) {
+ if (Opc == (*Likely ? Instruction::And : Instruction::Or))
+ // Its likely we will have to compute both lhs and rhs of condition
+ CostThresh += Params.LikelyBias;
+ else {
+ if (Params.UnlikelyBias < 0)
+ return false;
+ // Its likely we will get an early out.
+ CostThresh -= Params.UnlikelyBias;
+ }
+ }
+ }
+
+ if (CostThresh <= 0)
+ return false;
+
+ // Collect "all" instructions that lhs condition is dependent on.
+ SmallPtrSet<const Instruction *, 8> LhsDeps, RhsDeps;
+ collectInstructionDeps(&LhsDeps, Lhs);
+ // Collect "all" instructions that rhs condition is dependent on AND are
+ // dependencies of lhs. This gives us an estimate on which instructions we
+ // stand to save by splitting the condition.
+ if (!collectInstructionDeps(&RhsDeps, Rhs, &LhsDeps))
+ return false;
+ // Add the compare instruction itself unless its a dependency on the LHS.
+ if (const auto *RhsI = dyn_cast<Instruction>(Rhs))
+ if (!LhsDeps.contains(RhsI))
+ RhsDeps.insert(RhsI);
+
+ const auto &TLI = DAG.getTargetLoweringInfo();
+ const auto &TTI =
+ TLI.getTargetMachine().getTargetTransformInfo(*I.getFunction());
+
+ InstructionCost CostOfIncluding = 0;
+ // See if this instruction will need to computed independently of whether RHS
+ // is.
+ auto ShouldCountInsn = [&RhsDeps](const Instruction *Ins) {
+ for (const auto *U : Ins->users()) {
+ // If user is independent of RHS calculation we don't need to count it.
+ if (auto *UIns = dyn_cast<Instruction>(U))
+ if (!RhsDeps.contains(UIns))
----------------
goldsteinn wrote:
Realized there is a bug here, we should be checking that the use also doesn't equal `I.getCondition()`, otherwise we can end up overpruning. Have a fix, just testing if it implies any changes too the tuning. Will post tomorrow.
https://github.com/llvm/llvm-project/pull/81689
More information about the llvm-commits
mailing list