[llvm] [Analysis]: Allow inlining recursive call IF recursion depth is 1. (PR #119677)
Hassnaa Hamdi via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 17 11:08:33 PDT 2025
https://github.com/hassnaaHamdi updated https://github.com/llvm/llvm-project/pull/119677
>From efe2fc3dc3b2d311065eabe12b64ff8ee2dc01f4 Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Thu, 12 Dec 2024 08:48:17 +0000
Subject: [PATCH 1/7] [Analysis]: Allow inlining recursive call IF recursion
depth is 1.
---
llvm/include/llvm/Analysis/InlineCost.h | 2 +-
llvm/lib/Analysis/InlineCost.cpp | 80 ++++++++
.../Transforms/Inline/inline-recursive-fn.ll | 179 ++++++++++++++++++
llvm/test/Transforms/Inline/inline-remark.ll | 2 +-
4 files changed, 261 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/Transforms/Inline/inline-recursive-fn.ll
diff --git a/llvm/include/llvm/Analysis/InlineCost.h b/llvm/include/llvm/Analysis/InlineCost.h
index 90ee75773957a..2f398bf5157cf 100644
--- a/llvm/include/llvm/Analysis/InlineCost.h
+++ b/llvm/include/llvm/Analysis/InlineCost.h
@@ -236,7 +236,7 @@ struct InlineParams {
std::optional<bool> EnableDeferral;
/// Indicate whether we allow inlining for recursive call.
- std::optional<bool> AllowRecursiveCall = false;
+ std::optional<bool> AllowRecursiveCall = true;
};
std::optional<int> getStringFnAttrAsInt(CallBase &CB, StringRef AttrKind);
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index 30e1af602667c..46d31f2dd7a09 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -21,6 +21,7 @@
#include "llvm/Analysis/CodeMetrics.h"
#include "llvm/Analysis/ConstantFolding.h"
#include "llvm/Analysis/EphemeralValuesCache.h"
+#include "llvm/Analysis/DomConditionCache.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
@@ -1170,6 +1171,10 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
std::optional<CostBenefitPair> getCostBenefitPair() { return CostBenefit; }
bool wasDecidedByCostBenefit() const { return DecidedByCostBenefit; }
bool wasDecidedByCostThreshold() const { return DecidedByCostThreshold; }
+ bool shouldCheckRecursiveCall() {
+ return IsRecursiveCall && AllowRecursiveCall;
+ }
+ bool shouldInlineRecursiveCall(CallBase &Call);
};
// Return true if CB is the sole call to local function Callee.
@@ -2895,6 +2900,68 @@ InlineResult CallAnalyzer::analyze() {
return finalizeAnalysis();
}
+bool InlineCostCallAnalyzer::shouldInlineRecursiveCall(CallBase &Call) {
+ CallInst *CI = cast<CallInst>(&Call);
+ auto CIB = CI->getParent();
+ // Only handle case when we have sinlge predecessor
+ if (auto Predecessor = CIB->getSinglePredecessor()) {
+ BranchInst *Br = dyn_cast<BranchInst>(Predecessor->getTerminator());
+ if (!Br || Br->isUnconditional()) {
+ return false;
+ }
+ Value *Var = Br->getCondition();
+ CmpInst *CmpInstr = dyn_cast<CmpInst>(Var);
+ if (CmpInstr && !isa<Constant>(CmpInstr->getOperand(1))) {
+ // Current logic of ValueTracking/DomConditionCache works only if RHS is
+ // constant.
+ return false;
+ }
+ unsigned ArgNum = 0;
+ Value *FuncArg = nullptr, *CallArg = nullptr;
+ // Check which func argument the cmp instr is using:
+ for (; ArgNum < CI->getFunction()->arg_size(); ArgNum++) {
+ FuncArg = CI->getFunction()->getArg(ArgNum);
+ CallArg = CI->getArgOperand(ArgNum);
+ if (CmpInstr) {
+ if ((FuncArg == CmpInstr->getOperand(0)) &&
+ (CallArg != CmpInstr->getOperand(0)))
+ break;
+ } else if (FuncArg == Var && (CallArg != Var))
+ break;
+ }
+ // Only handle the case when a func argument controls the cmp instruction:
+ if (ArgNum < CI->getFunction()->arg_size()) {
+ bool isTrueSuccessor = CIB == Br->getSuccessor(0);
+ if (CmpInstr) {
+ SimplifyQuery SQ(CI->getFunction()->getDataLayout(),
+ dyn_cast<Instruction>(CallArg));
+ DomConditionCache DC;
+ DC.registerBranch(Br);
+ SQ.DC = &DC;
+ DominatorTree DT(*CI->getFunction());
+ SQ.DT = &DT;
+ Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(
+ CmpInstr, {CallArg, CmpInstr->getOperand(1)}, SQ);
+ if (!simplifiedInstruction)
+ return false;
+ if (auto *ConstVal =
+ dyn_cast<llvm::ConstantInt>(simplifiedInstruction)) {
+ if (ConstVal->isOne())
+ return !isTrueSuccessor;
+ return isTrueSuccessor;
+ }
+ } else {
+ if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(CallArg)) {
+ if (ConstVal->isOne())
+ return !isTrueSuccessor;
+ return isTrueSuccessor;
+ }
+ }
+ }
+ }
+ return false;
+}
+
void InlineCostCallAnalyzer::print(raw_ostream &OS) {
#define DEBUG_PRINT_STAT(x) OS << " " #x ": " << x << "\n"
if (PrintInstructionComments)
@@ -3126,6 +3193,12 @@ InlineCost llvm::getInlineCost(
LLVM_DEBUG(CA.dump());
+ // Check if callee function is recursive:
+ if (ShouldInline.isSuccess()) {
+ if (CA.shouldCheckRecursiveCall() && !CA.shouldInlineRecursiveCall(Call))
+ return InlineCost::getNever("deep recursion");
+ }
+
// Always make cost benefit based decision explicit.
// We use always/never here since threshold is not meaningful,
// as it's not what drives cost-benefit analysis.
@@ -3168,6 +3241,13 @@ InlineResult llvm::isInlineViable(Function &F) {
// Disallow recursive calls.
Function *Callee = Call->getCalledFunction();
+ // This function is called when we have "alwaysinline" attribute.
+ // If we allowed the inlining here given that the recursive inlining is
+ // allowed, then there will be problem in the second trial of inlining,
+ // because the Inliner pass allow only one time inlining and then it
+ // inserts "noinline" attribute which will be in conflict with the
+ // attribute of "alwaysinline" so, "alwaysinline" for recursive function
+ // will be disallowed to avoid conflict of attributes.
if (&F == Callee)
return InlineResult::failure("recursive call");
diff --git a/llvm/test/Transforms/Inline/inline-recursive-fn.ll b/llvm/test/Transforms/Inline/inline-recursive-fn.ll
new file mode 100644
index 0000000000000..db90050d009a0
--- /dev/null
+++ b/llvm/test/Transforms/Inline/inline-recursive-fn.ll
@@ -0,0 +1,179 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes='inline,instcombine' < %s | FileCheck %s
+
+define float @inline_rec_true_successor(float %x, float %scale) {
+; CHECK-LABEL: define float @inline_rec_true_successor(
+; CHECK-SAME: float [[X:%.*]], float [[SCALE:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[CMP:%.*]] = fcmp olt float [[X]], 0.000000e+00
+; CHECK-NEXT: br i1 [[CMP]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; CHECK: [[COMMON_RET18:.*]]:
+; CHECK-NEXT: [[COMMON_RET18_OP:%.*]] = phi float [ [[COMMON_RET18_OP_I:%.*]], %[[INLINE_REC_TRUE_SUCCESSOR_EXIT:.*]] ], [ [[MUL:%.*]], %[[IF_END]] ]
+; CHECK-NEXT: ret float [[COMMON_RET18_OP]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: br i1 false, label %[[IF_THEN_I:.*]], label %[[IF_END_I:.*]]
+; CHECK: [[IF_THEN_I]]:
+; CHECK-NEXT: br label %[[INLINE_REC_TRUE_SUCCESSOR_EXIT]]
+; CHECK: [[IF_END_I]]:
+; CHECK-NEXT: [[FNEG:%.*]] = fneg float [[X]]
+; CHECK-NEXT: [[MUL_I:%.*]] = fmul float [[SCALE]], [[FNEG]]
+; CHECK-NEXT: br label %[[INLINE_REC_TRUE_SUCCESSOR_EXIT]]
+; CHECK: [[INLINE_REC_TRUE_SUCCESSOR_EXIT]]:
+; CHECK-NEXT: [[COMMON_RET18_OP_I]] = phi float [ poison, %[[IF_THEN_I]] ], [ [[MUL_I]], %[[IF_END_I]] ]
+; CHECK-NEXT: br label %[[COMMON_RET18]]
+; CHECK: [[IF_END]]:
+; CHECK-NEXT: [[MUL]] = fmul float [[X]], [[SCALE]]
+; CHECK-NEXT: br label %[[COMMON_RET18]]
+;
+entry:
+ %cmp = fcmp olt float %x, 0.000000e+00
+ br i1 %cmp, label %if.then, label %if.end
+
+common.ret18: ; preds = %if.then, %if.end
+ %common.ret18.op = phi float [ %call, %if.then ], [ %mul, %if.end ]
+ ret float %common.ret18.op
+
+if.then: ; preds = %entry
+ %fneg = fneg float %x
+ %call = tail call float @inline_rec_true_successor(float %fneg, float %scale)
+ br label %common.ret18
+
+if.end: ; preds = %entry
+ %mul = fmul float %x, %scale
+ br label %common.ret18
+}
+
+define float @test_inline_rec_true_successor(float %x, float %scale) {
+entry:
+ %res = tail call float @inline_rec_true_successor(float %x, float %scale)
+ ret float %res
+}
+
+; Same as previous test except that the recursive callsite is in the false successor
+define float @inline_rec_false_successor(float %x, float %scale) {
+; CHECK-LABEL: define float @inline_rec_false_successor(
+; CHECK-SAME: float [[Y:%.*]], float [[SCALE:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[CMP:%.*]] = fcmp uge float [[Y]], 0.000000e+00
+; CHECK-NEXT: br i1 [[CMP]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; CHECK: [[COMMON_RET18:.*]]:
+; CHECK-NEXT: [[COMMON_RET18_OP:%.*]] = phi float [ [[MUL:%.*]], %[[IF_THEN]] ], [ [[COMMON_RET18_OP_I:%.*]], %[[INLINE_REC_FALSE_SUCCESSOR_EXIT:.*]] ]
+; CHECK-NEXT: ret float [[COMMON_RET18_OP]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: [[MUL]] = fmul float [[Y]], [[SCALE]]
+; CHECK-NEXT: br label %[[COMMON_RET18]]
+; CHECK: [[IF_END]]:
+; CHECK-NEXT: br i1 true, label %[[IF_THEN_I:.*]], label %[[IF_END_I:.*]]
+; CHECK: [[IF_THEN_I]]:
+; CHECK-NEXT: [[FNEG:%.*]] = fneg float [[Y]]
+; CHECK-NEXT: [[MUL_I:%.*]] = fmul float [[SCALE]], [[FNEG]]
+; CHECK-NEXT: br label %[[INLINE_REC_FALSE_SUCCESSOR_EXIT]]
+; CHECK: [[IF_END_I]]:
+; CHECK-NEXT: br label %[[INLINE_REC_FALSE_SUCCESSOR_EXIT]]
+; CHECK: [[INLINE_REC_FALSE_SUCCESSOR_EXIT]]:
+; CHECK-NEXT: [[COMMON_RET18_OP_I]] = phi float [ [[MUL_I]], %[[IF_THEN_I]] ], [ poison, %[[IF_END_I]] ]
+; CHECK-NEXT: br label %[[COMMON_RET18]]
+;
+entry:
+ %cmp = fcmp uge float %x, 0.000000e+00
+ br i1 %cmp, label %if.then, label %if.end
+
+common.ret18: ; preds = %if.then, %if.end
+ %common.ret18.op = phi float [ %mul, %if.then ], [ %call, %if.end ]
+ ret float %common.ret18.op
+
+if.then: ; preds = %entry
+ %mul = fmul float %x, %scale
+ br label %common.ret18
+
+if.end: ; preds = %entry
+ %fneg = fneg float %x
+ %call = tail call float @inline_rec_false_successor(float %fneg, float %scale)
+ br label %common.ret18
+}
+
+define float @test_inline_rec_false_successor(float %x, float %scale) {
+entry:
+ %res = tail call float @inline_rec_false_successor(float %x, float %scale)
+ ret float %res
+}
+
+; Test when the BR has Value not cmp instruction
+define float @inline_rec_no_cmp(i1 %flag, float %scale) {
+; CHECK-LABEL: define float @inline_rec_no_cmp(
+; CHECK-SAME: i1 [[FLAG:%.*]], float [[SCALE:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br i1 [[FLAG]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: [[SUM:%.*]] = fadd float [[SCALE]], 5.000000e+00
+; CHECK-NEXT: [[SUM1:%.*]] = fadd float [[SUM]], [[SCALE]]
+; CHECK-NEXT: br label %[[COMMON_RET:.*]]
+; CHECK: [[IF_END]]:
+; CHECK-NEXT: [[SUM2:%.*]] = fadd float [[SCALE]], 5.000000e+00
+; CHECK-NEXT: br label %[[COMMON_RET]]
+; CHECK: [[COMMON_RET]]:
+; CHECK-NEXT: [[COMMON_RET_RES:%.*]] = phi float [ [[SUM1]], %[[IF_THEN]] ], [ [[SUM2]], %[[IF_END]] ]
+; CHECK-NEXT: ret float [[COMMON_RET_RES]]
+;
+entry:
+ br i1 %flag, label %if.then, label %if.end
+if.then:
+ %res = tail call float @inline_rec_no_cmp(i1 false, float %scale)
+ %sum1 = fadd float %res, %scale
+ br label %common.ret
+if.end:
+ %sum2 = fadd float %scale, 5.000000e+00
+ br label %common.ret
+common.ret:
+ %common.ret.res = phi float [ %sum1, %if.then ], [ %sum2, %if.end ]
+ ret float %common.ret.res
+}
+
+define float @test_inline_rec_no_cmp(i1 %flag, float %scale) {
+entry:
+ %res = tail call float @inline_rec_no_cmp(i1 %flag, float %scale)
+ ret float %res
+}
+
+define float @no_inline_rec(float %x, float %scale) {
+; CHECK-LABEL: define float @no_inline_rec(
+; CHECK-SAME: float [[Z:%.*]], float [[SCALE:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[CMP:%.*]] = fcmp olt float [[Z]], 5.000000e+00
+; CHECK-NEXT: br i1 [[CMP]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; CHECK: [[COMMON_RET18:.*]]:
+; CHECK-NEXT: [[COMMON_RET18_OP:%.*]] = phi float [ [[FNEG1:%.*]], %[[IF_THEN]] ], [ [[MUL:%.*]], %[[IF_END]] ]
+; CHECK-NEXT: ret float [[COMMON_RET18_OP]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: [[FADD:%.*]] = fadd float [[Z]], 5.000000e+00
+; CHECK-NEXT: [[CALL:%.*]] = tail call float @no_inline_rec(float [[FADD]], float [[SCALE]])
+; CHECK-NEXT: [[FNEG1]] = fneg float [[CALL]]
+; CHECK-NEXT: br label %[[COMMON_RET18]]
+; CHECK: [[IF_END]]:
+; CHECK-NEXT: [[MUL]] = fmul float [[Z]], [[SCALE]]
+; CHECK-NEXT: br label %[[COMMON_RET18]]
+;
+entry:
+ %cmp = fcmp olt float %x, 5.000000e+00
+ br i1 %cmp, label %if.then, label %if.end
+
+common.ret18: ; preds = %if.then, %if.end
+ %common.ret18.op = phi float [ %fneg1, %if.then ], [ %mul, %if.end ]
+ ret float %common.ret18.op
+
+if.then: ; preds = %entry
+ %fadd = fadd float %x, 5.000000e+00
+ %call = tail call float @no_inline_rec(float %fadd, float %scale)
+ %fneg1 = fneg float %call
+ br label %common.ret18
+
+if.end: ; preds = %entry
+ %mul = fmul float %x, %scale
+ br label %common.ret18
+}
+
+define float @test_no_inline(float %x, float %scale) {
+entry:
+ %res = tail call float @no_inline_rec(float %x, float %scale)
+ ret float %res
+}
\ No newline at end of file
diff --git a/llvm/test/Transforms/Inline/inline-remark.ll b/llvm/test/Transforms/Inline/inline-remark.ll
index 166c7bb065924..5af20e42dd31a 100644
--- a/llvm/test/Transforms/Inline/inline-remark.ll
+++ b/llvm/test/Transforms/Inline/inline-remark.ll
@@ -56,6 +56,6 @@ define void @test3() {
}
; CHECK: attributes [[ATTR1]] = { "inline-remark"="(cost=25, threshold=0)" }
-; CHECK: attributes [[ATTR2]] = { "inline-remark"="(cost=never): recursive" }
+; CHECK: attributes [[ATTR2]] = { "inline-remark"="(cost=0, threshold=0)" }
; CHECK: attributes [[ATTR3]] = { "inline-remark"="unsupported operand bundle; (cost={{.*}}, threshold={{.*}})" }
; CHECK: attributes [[ATTR4]] = { alwaysinline "inline-remark"="(cost=never): recursive call" }
>From ef313051bb1346dfadd1d81eb13d460bee67b1b8 Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Tue, 17 Dec 2024 17:32:10 +0000
Subject: [PATCH 2/7] [InlineCost]: fold CmpInstr when next iteration of a
recursive call can simplify the Cmp decision.
---
llvm/include/llvm/Analysis/InlineCost.h | 2 +-
llvm/lib/Analysis/InlineCost.cpp | 146 +++++++++----------
llvm/test/Transforms/Inline/inline-remark.ll | 2 +-
3 files changed, 69 insertions(+), 81 deletions(-)
diff --git a/llvm/include/llvm/Analysis/InlineCost.h b/llvm/include/llvm/Analysis/InlineCost.h
index 2f398bf5157cf..90ee75773957a 100644
--- a/llvm/include/llvm/Analysis/InlineCost.h
+++ b/llvm/include/llvm/Analysis/InlineCost.h
@@ -236,7 +236,7 @@ struct InlineParams {
std::optional<bool> EnableDeferral;
/// Indicate whether we allow inlining for recursive call.
- std::optional<bool> AllowRecursiveCall = true;
+ std::optional<bool> AllowRecursiveCall = false;
};
std::optional<int> getStringFnAttrAsInt(CallBase &CB, StringRef AttrKind);
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index 46d31f2dd7a09..75855823f11e8 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -445,6 +445,7 @@ class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
bool canFoldInboundsGEP(GetElementPtrInst &I);
bool accumulateGEPOffset(GEPOperator &GEP, APInt &Offset);
bool simplifyCallSite(Function *F, CallBase &Call);
+ bool simplifyCmpInst(Function *F, CmpInst &Cmp);
bool simplifyInstruction(Instruction &I);
bool simplifyIntrinsicCallIsConstant(CallBase &CB);
bool simplifyIntrinsicCallObjectSize(CallBase &CB);
@@ -1171,10 +1172,6 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
std::optional<CostBenefitPair> getCostBenefitPair() { return CostBenefit; }
bool wasDecidedByCostBenefit() const { return DecidedByCostBenefit; }
bool wasDecidedByCostThreshold() const { return DecidedByCostThreshold; }
- bool shouldCheckRecursiveCall() {
- return IsRecursiveCall && AllowRecursiveCall;
- }
- bool shouldInlineRecursiveCall(CallBase &Call);
};
// Return true if CB is the sole call to local function Callee.
@@ -1681,6 +1678,68 @@ bool CallAnalyzer::visitGetElementPtr(GetElementPtrInst &I) {
return isGEPFree(I);
}
+/// Simplify \p Cmp if RHS is const and we can ValueTrack LHS,
+// This handles the case when the Cmp instruction is guarded a recursive call
+// that will cause the Cmp to fail/succeed for the next iteration.
+bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
+ // Bail out if the RHS is NOT const:
+ if (!isa<Constant>(Cmp.getOperand(1)))
+ return false;
+ auto *CmpOp = Cmp.getOperand(0);
+ // Iterate over the users of the function to check if it's a recursive function:
+ for (auto *U : F->users()) {
+ CallInst *Call = dyn_cast<CallInst>(U);
+ if (!Call || Call->getFunction() != F)
+ continue;
+ auto *CallBB = Call->getParent();
+ auto *Predecessor = CallBB->getSinglePredecessor();
+ // Only handle the case when the callsite has a single predecessor:
+ if (!Predecessor)
+ continue;
+
+ auto *Br = dyn_cast<BranchInst>(Predecessor->getTerminator());
+ if (!Br || Br->isUnconditional())
+ continue;
+ // Check if the Br condition is the same Cmp instr we are investigating:
+ auto *CmpInstr = dyn_cast<CmpInst>(Br->getCondition());
+ if (!CmpInstr || CmpInstr != &Cmp)
+ continue;
+ // Check if there are any arg of the recursive callsite is affecting the cmp instr:
+ bool ArgFound = false;
+ Value *FuncArg = nullptr, *CallArg = nullptr;
+ for (unsigned ArgNum = 0; ArgNum < F->arg_size() && ArgNum < Call->arg_size(); ArgNum ++) {
+ FuncArg = F->getArg(ArgNum);
+ CallArg = Call->getArgOperand(ArgNum);
+ if ((FuncArg == CmpOp) &&
+ (CallArg != CmpOp)) {
+ ArgFound = true;
+ break;
+ }
+ }
+ if (!ArgFound)
+ continue;
+ // Now we have a recursive call that is guarded by a cmp instruction.
+ // Check if this cmp can be simplified:
+ SimplifyQuery SQ(DL, dyn_cast<Instruction>(CallArg));
+ DomConditionCache DC;
+ DC.registerBranch(Br);
+ SQ.DC = &DC;
+ DominatorTree DT(*F);
+ SQ.DT = &DT;
+ Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(CmpInstr, {CallArg, Cmp.getOperand(1)}, SQ);
+ if (!simplifiedInstruction)
+ continue;
+ if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(simplifiedInstruction)) {
+ bool isTrueSuccessor = CallBB == Br->getSuccessor(0);
+ SimplifiedValues[&Cmp] = ConstVal;
+ if (ConstVal->isOne())
+ return !isTrueSuccessor;
+ return isTrueSuccessor;
+ }
+ }
+ return false;
+}
+
/// Simplify \p I if its operands are constants and update SimplifiedValues.
bool CallAnalyzer::simplifyInstruction(Instruction &I) {
SmallVector<Constant *> COps;
@@ -2065,6 +2124,10 @@ bool CallAnalyzer::visitCmpInst(CmpInst &I) {
if (simplifyInstruction(I))
return true;
+ // Try to handle comparison that can be simplified using ValueTracking.
+ if (simplifyCmpInst(I.getFunction(), I))
+ return true;
+
if (I.getOpcode() == Instruction::FCmp)
return false;
@@ -2900,68 +2963,6 @@ InlineResult CallAnalyzer::analyze() {
return finalizeAnalysis();
}
-bool InlineCostCallAnalyzer::shouldInlineRecursiveCall(CallBase &Call) {
- CallInst *CI = cast<CallInst>(&Call);
- auto CIB = CI->getParent();
- // Only handle case when we have sinlge predecessor
- if (auto Predecessor = CIB->getSinglePredecessor()) {
- BranchInst *Br = dyn_cast<BranchInst>(Predecessor->getTerminator());
- if (!Br || Br->isUnconditional()) {
- return false;
- }
- Value *Var = Br->getCondition();
- CmpInst *CmpInstr = dyn_cast<CmpInst>(Var);
- if (CmpInstr && !isa<Constant>(CmpInstr->getOperand(1))) {
- // Current logic of ValueTracking/DomConditionCache works only if RHS is
- // constant.
- return false;
- }
- unsigned ArgNum = 0;
- Value *FuncArg = nullptr, *CallArg = nullptr;
- // Check which func argument the cmp instr is using:
- for (; ArgNum < CI->getFunction()->arg_size(); ArgNum++) {
- FuncArg = CI->getFunction()->getArg(ArgNum);
- CallArg = CI->getArgOperand(ArgNum);
- if (CmpInstr) {
- if ((FuncArg == CmpInstr->getOperand(0)) &&
- (CallArg != CmpInstr->getOperand(0)))
- break;
- } else if (FuncArg == Var && (CallArg != Var))
- break;
- }
- // Only handle the case when a func argument controls the cmp instruction:
- if (ArgNum < CI->getFunction()->arg_size()) {
- bool isTrueSuccessor = CIB == Br->getSuccessor(0);
- if (CmpInstr) {
- SimplifyQuery SQ(CI->getFunction()->getDataLayout(),
- dyn_cast<Instruction>(CallArg));
- DomConditionCache DC;
- DC.registerBranch(Br);
- SQ.DC = &DC;
- DominatorTree DT(*CI->getFunction());
- SQ.DT = &DT;
- Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(
- CmpInstr, {CallArg, CmpInstr->getOperand(1)}, SQ);
- if (!simplifiedInstruction)
- return false;
- if (auto *ConstVal =
- dyn_cast<llvm::ConstantInt>(simplifiedInstruction)) {
- if (ConstVal->isOne())
- return !isTrueSuccessor;
- return isTrueSuccessor;
- }
- } else {
- if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(CallArg)) {
- if (ConstVal->isOne())
- return !isTrueSuccessor;
- return isTrueSuccessor;
- }
- }
- }
- }
- return false;
-}
-
void InlineCostCallAnalyzer::print(raw_ostream &OS) {
#define DEBUG_PRINT_STAT(x) OS << " " #x ": " << x << "\n"
if (PrintInstructionComments)
@@ -3193,12 +3194,6 @@ InlineCost llvm::getInlineCost(
LLVM_DEBUG(CA.dump());
- // Check if callee function is recursive:
- if (ShouldInline.isSuccess()) {
- if (CA.shouldCheckRecursiveCall() && !CA.shouldInlineRecursiveCall(Call))
- return InlineCost::getNever("deep recursion");
- }
-
// Always make cost benefit based decision explicit.
// We use always/never here since threshold is not meaningful,
// as it's not what drives cost-benefit analysis.
@@ -3241,13 +3236,6 @@ InlineResult llvm::isInlineViable(Function &F) {
// Disallow recursive calls.
Function *Callee = Call->getCalledFunction();
- // This function is called when we have "alwaysinline" attribute.
- // If we allowed the inlining here given that the recursive inlining is
- // allowed, then there will be problem in the second trial of inlining,
- // because the Inliner pass allow only one time inlining and then it
- // inserts "noinline" attribute which will be in conflict with the
- // attribute of "alwaysinline" so, "alwaysinline" for recursive function
- // will be disallowed to avoid conflict of attributes.
if (&F == Callee)
return InlineResult::failure("recursive call");
diff --git a/llvm/test/Transforms/Inline/inline-remark.ll b/llvm/test/Transforms/Inline/inline-remark.ll
index 5af20e42dd31a..166c7bb065924 100644
--- a/llvm/test/Transforms/Inline/inline-remark.ll
+++ b/llvm/test/Transforms/Inline/inline-remark.ll
@@ -56,6 +56,6 @@ define void @test3() {
}
; CHECK: attributes [[ATTR1]] = { "inline-remark"="(cost=25, threshold=0)" }
-; CHECK: attributes [[ATTR2]] = { "inline-remark"="(cost=0, threshold=0)" }
+; CHECK: attributes [[ATTR2]] = { "inline-remark"="(cost=never): recursive" }
; CHECK: attributes [[ATTR3]] = { "inline-remark"="unsupported operand bundle; (cost={{.*}}, threshold={{.*}})" }
; CHECK: attributes [[ATTR4]] = { alwaysinline "inline-remark"="(cost=never): recursive call" }
>From 48cbe62ee004962de465f2ec088a10f8f596aa59 Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Tue, 17 Dec 2024 17:32:44 +0000
Subject: [PATCH 3/7] format
---
llvm/lib/Analysis/InlineCost.cpp | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index 75855823f11e8..83802de69f11a 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -1686,7 +1686,8 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
if (!isa<Constant>(Cmp.getOperand(1)))
return false;
auto *CmpOp = Cmp.getOperand(0);
- // Iterate over the users of the function to check if it's a recursive function:
+ // Iterate over the users of the function to check if it's a recursive
+ // function:
for (auto *U : F->users()) {
CallInst *Call = dyn_cast<CallInst>(U);
if (!Call || Call->getFunction() != F)
@@ -1704,14 +1705,15 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
auto *CmpInstr = dyn_cast<CmpInst>(Br->getCondition());
if (!CmpInstr || CmpInstr != &Cmp)
continue;
- // Check if there are any arg of the recursive callsite is affecting the cmp instr:
+ // Check if there are any arg of the recursive callsite is affecting the cmp
+ // instr:
bool ArgFound = false;
Value *FuncArg = nullptr, *CallArg = nullptr;
- for (unsigned ArgNum = 0; ArgNum < F->arg_size() && ArgNum < Call->arg_size(); ArgNum ++) {
+ for (unsigned ArgNum = 0;
+ ArgNum < F->arg_size() && ArgNum < Call->arg_size(); ArgNum++) {
FuncArg = F->getArg(ArgNum);
CallArg = Call->getArgOperand(ArgNum);
- if ((FuncArg == CmpOp) &&
- (CallArg != CmpOp)) {
+ if ((FuncArg == CmpOp) && (CallArg != CmpOp)) {
ArgFound = true;
break;
}
@@ -1726,7 +1728,8 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
SQ.DC = &DC;
DominatorTree DT(*F);
SQ.DT = &DT;
- Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(CmpInstr, {CallArg, Cmp.getOperand(1)}, SQ);
+ Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(
+ CmpInstr, {CallArg, Cmp.getOperand(1)}, SQ);
if (!simplifiedInstruction)
continue;
if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(simplifiedInstruction)) {
>From 6afc48f0f3594fc79f7a11d2b4f6c7d0129a04bf Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Wed, 26 Feb 2025 00:38:19 +0000
Subject: [PATCH 4/7] Update DT only when we get different functions
Change-Id: I296a55dba127c1ae74cca9e0edcb10f2f44640e9
---
llvm/lib/Analysis/InlineCost.cpp | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index 83802de69f11a..fd2063192acf3 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -263,6 +263,8 @@ class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
// Cache the DataLayout since we use it a lot.
const DataLayout &DL;
+ DominatorTree DT;
+
/// The OptimizationRemarkEmitter available for this compilation.
OptimizationRemarkEmitter *ORE;
@@ -1726,7 +1728,14 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
DomConditionCache DC;
DC.registerBranch(Br);
SQ.DC = &DC;
- DominatorTree DT(*F);
+ if (DT.root_size() == 0) {
+ // Dominator tree was never constructed for any function yet.
+ DT.recalculate(*F);
+ } else if (DT.getRoot()->getParent() != F) {
+ // Dominator tree was constructed for a different function, recalculate
+ // it for the current function.
+ DT.recalculate(*F);
+ }
SQ.DT = &DT;
Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(
CmpInstr, {CallArg, Cmp.getOperand(1)}, SQ);
>From d5ff993289ae1953d92bf407bec20adffeeabe12 Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Mon, 7 Apr 2025 11:00:09 +0000
Subject: [PATCH 5/7] resolve review comments
---
llvm/lib/Analysis/InlineCost.cpp | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index fd2063192acf3..c4970cc6455a3 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -1680,19 +1680,19 @@ bool CallAnalyzer::visitGetElementPtr(GetElementPtrInst &I) {
return isGEPFree(I);
}
-/// Simplify \p Cmp if RHS is const and we can ValueTrack LHS,
-// This handles the case when the Cmp instruction is guarded a recursive call
-// that will cause the Cmp to fail/succeed for the next iteration.
+// Simplify \p Cmp if RHS is const and we can ValueTrack LHS.
+// This handles the case when the Cmp instruction is guarding a recursive call
+// that will cause the Cmp to fail/succeed for the recursive call.
bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
- // Bail out if the RHS is NOT const:
- if (!isa<Constant>(Cmp.getOperand(1)))
+ // Bail out if LHS is not a function argument or RHS is NOT const:
+ if (!isa<Argument>(Cmp.getOperand(0)) || !isa<Constant>(Cmp.getOperand(1)))
return false;
auto *CmpOp = Cmp.getOperand(0);
// Iterate over the users of the function to check if it's a recursive
// function:
for (auto *U : F->users()) {
CallInst *Call = dyn_cast<CallInst>(U);
- if (!Call || Call->getFunction() != F)
+ if (!Call || Call->getFunction() != F || Call->getCalledFunction() != F)
continue;
auto *CallBB = Call->getParent();
auto *Predecessor = CallBB->getSinglePredecessor();
@@ -1704,8 +1704,7 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
if (!Br || Br->isUnconditional())
continue;
// Check if the Br condition is the same Cmp instr we are investigating:
- auto *CmpInstr = dyn_cast<CmpInst>(Br->getCondition());
- if (!CmpInstr || CmpInstr != &Cmp)
+ if (Br->getCondition() != &Cmp)
continue;
// Check if there are any arg of the recursive callsite is affecting the cmp
// instr:
@@ -1715,7 +1714,7 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
ArgNum < F->arg_size() && ArgNum < Call->arg_size(); ArgNum++) {
FuncArg = F->getArg(ArgNum);
CallArg = Call->getArgOperand(ArgNum);
- if ((FuncArg == CmpOp) && (CallArg != CmpOp)) {
+ if (FuncArg == CmpOp && CallArg != CmpOp) {
ArgFound = true;
break;
}
@@ -1737,16 +1736,16 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
DT.recalculate(*F);
}
SQ.DT = &DT;
- Value *simplifiedInstruction = llvm::simplifyInstructionWithOperands(
- CmpInstr, {CallArg, Cmp.getOperand(1)}, SQ);
- if (!simplifiedInstruction)
+ Value *SimplifiedInstruction = llvm::simplifyInstructionWithOperands(
+ cast<CmpInst>(&Cmp), {CallArg, Cmp.getOperand(1)}, SQ);
+ if (!SimplifiedInstruction)
continue;
- if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(simplifiedInstruction)) {
- bool isTrueSuccessor = CallBB == Br->getSuccessor(0);
+ if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(SimplifiedInstruction)) {
+ bool IsTrueSuccessor = CallBB == Br->getSuccessor(0);
SimplifiedValues[&Cmp] = ConstVal;
if (ConstVal->isOne())
- return !isTrueSuccessor;
- return isTrueSuccessor;
+ return !IsTrueSuccessor;
+ return IsTrueSuccessor;
}
}
return false;
>From d6642e2e7c56830da78bdcd9d7eed433f167fccb Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Mon, 7 Apr 2025 12:14:44 +0000
Subject: [PATCH 6/7] fix format
---
llvm/lib/Analysis/InlineCost.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index c4970cc6455a3..d689227ede502 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -20,8 +20,8 @@
#include "llvm/Analysis/BlockFrequencyInfo.h"
#include "llvm/Analysis/CodeMetrics.h"
#include "llvm/Analysis/ConstantFolding.h"
-#include "llvm/Analysis/EphemeralValuesCache.h"
#include "llvm/Analysis/DomConditionCache.h"
+#include "llvm/Analysis/EphemeralValuesCache.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
>From c5003a603e660f6ec1cb8fda57d19e9ef497a8b6 Mon Sep 17 00:00:00 2001
From: Hassnaa Hamdi <hassnaa.hamdi at arm.com>
Date: Thu, 17 Apr 2025 16:41:01 +0000
Subject: [PATCH 7/7] resolve review comments
---
llvm/lib/Analysis/InlineCost.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index d689227ede502..3740b48528640 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -1738,9 +1738,7 @@ bool CallAnalyzer::simplifyCmpInst(Function *F, CmpInst &Cmp) {
SQ.DT = &DT;
Value *SimplifiedInstruction = llvm::simplifyInstructionWithOperands(
cast<CmpInst>(&Cmp), {CallArg, Cmp.getOperand(1)}, SQ);
- if (!SimplifiedInstruction)
- continue;
- if (auto *ConstVal = dyn_cast<llvm::ConstantInt>(SimplifiedInstruction)) {
+ if (auto *ConstVal = dyn_cast_or_null<ConstantInt>(SimplifiedInstruction)) {
bool IsTrueSuccessor = CallBB == Br->getSuccessor(0);
SimplifiedValues[&Cmp] = ConstVal;
if (ConstVal->isOne())
More information about the llvm-commits
mailing list