[llvm] [Analysis]: Allow inlining recursive call IF recursion depth is 1. (PR #119677)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 12 01:01:09 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: Hassnaa Hamdi (hassnaaHamdi)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/119677.diff
4 Files Affected:
- (modified) llvm/include/llvm/Analysis/InlineCost.h (+1-1)
- (modified) llvm/lib/Analysis/InlineCost.cpp (+80)
- (added) llvm/test/Transforms/Inline/inline-recursive-fn.ll (+179)
- (modified) llvm/test/Transforms/Inline/inline-remark.ll (+1-1)
``````````diff
diff --git a/llvm/include/llvm/Analysis/InlineCost.h b/llvm/include/llvm/Analysis/InlineCost.h
index ed54b0c077b4a4..ac61ca064c28b3 100644
--- a/llvm/include/llvm/Analysis/InlineCost.h
+++ b/llvm/include/llvm/Analysis/InlineCost.h
@@ -235,7 +235,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 85287a39f2caad..c2ade73b2ddcb4 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -20,6 +20,7 @@
#include "llvm/Analysis/BlockFrequencyInfo.h"
#include "llvm/Analysis/CodeMetrics.h"
#include "llvm/Analysis/ConstantFolding.h"
+#include "llvm/Analysis/DomConditionCache.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
@@ -1160,6 +1161,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.
@@ -2880,6 +2885,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)
@@ -3106,6 +3173,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.
@@ -3148,6 +3221,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 00000000000000..db90050d009a04
--- /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 166c7bb0659246..5af20e42dd31a1 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" }
``````````
</details>
https://github.com/llvm/llvm-project/pull/119677
More information about the llvm-commits
mailing list