[llvm] e240678 - [ArgPromotion] Protect harder against recursive promotion (PR42028)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 11 00:30:46 PST 2022
Author: Nikita Popov
Date: 2022-02-11T09:30:39+01:00
New Revision: e24067819fbda2f2bf5d1f43dfe36c114accc21d
URL: https://github.com/llvm/llvm-project/commit/e24067819fbda2f2bf5d1f43dfe36c114accc21d
DIFF: https://github.com/llvm/llvm-project/commit/e24067819fbda2f2bf5d1f43dfe36c114accc21d.diff
LOG: [ArgPromotion] Protect harder against recursive promotion (PR42028)
In addition to the self-recursion check, also check whether there
is more than one node in the SCC, which implies that there is a
larger cycle. I believe checking SCC structure (rather than
something like norecurse) is the right thing to do here, because
this is specifically about preventing infinite loops over the SCC.
Fixes https://github.com/llvm/llvm-project/issues/42028.
Differential Revision: https://reviews.llvm.org/D119418
Added:
llvm/test/Transforms/ArgumentPromotion/pr42028-recursion.ll
Modified:
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 509f0166a9a43..75cd582fdff96 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -454,7 +454,7 @@ static bool allCallersPassValidPointerForArgument(Argument *Arg,
/// Determine that this argument is safe to promote, and find the argument
/// parts it can be promoted into.
static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
- unsigned MaxElements, bool IsSelfRecursive,
+ unsigned MaxElements, bool IsRecursive,
SmallVectorImpl<OffsetAndArgPart> &ArgPartsVec) {
// Quick exit for unused arguments
if (Arg->use_empty())
@@ -501,9 +501,9 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
if (Size.isScalable())
return false;
- // If this is a self-recursive function and one of the types is a pointer,
+ // If this is a recursive function and one of the types is a pointer,
// then promoting it might lead to recursive promotion.
- if (IsSelfRecursive && Ty->isPointerTy())
+ if (IsRecursive && Ty->isPointerTy())
return false;
int64_t Off = Offset.getSExtValue();
@@ -760,7 +760,7 @@ promoteArguments(Function *F, function_ref<AAResults &(Function &F)> AARGetter,
unsigned MaxElements,
Optional<function_ref<void(CallBase &OldCS, CallBase &NewCS)>>
ReplaceCallSite,
- const TargetTransformInfo &TTI) {
+ const TargetTransformInfo &TTI, bool IsRecursive) {
// Don't perform argument promotion for naked functions; otherwise we can end
// up removing parameters that are seemingly 'not used' as they are referred
// to in the assembly.
@@ -794,8 +794,7 @@ promoteArguments(Function *F, function_ref<AAResults &(Function &F)> AARGetter,
// Second check: make sure that all callers are direct callers. We can't
// transform functions that have indirect callers. Also see if the function
- // is self-recursive and check that target features are compatible.
- bool isSelfRecursive = false;
+ // is self-recursive.
for (Use &U : F->uses()) {
CallBase *CB = dyn_cast<CallBase>(U.getUser());
// Must be a direct call.
@@ -807,7 +806,7 @@ promoteArguments(Function *F, function_ref<AAResults &(Function &F)> AARGetter,
return nullptr;
if (CB->getParent()->getParent() == F)
- isSelfRecursive = true;
+ IsRecursive = true;
}
// Can't change signature of musttail caller
@@ -879,7 +878,7 @@ promoteArguments(Function *F, function_ref<AAResults &(Function &F)> AARGetter,
// Otherwise, see if we can promote the pointer to its value.
SmallVector<OffsetAndArgPart, 4> ArgParts;
- if (findArgParts(PtrArg, DL, AAR, MaxElements, isSelfRecursive, ArgParts)) {
+ if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts)) {
SmallVector<Type *, 4> Types;
for (const auto &Pair : ArgParts)
Types.push_back(Pair.second.Ty);
@@ -909,6 +908,7 @@ PreservedAnalyses ArgumentPromotionPass::run(LazyCallGraph::SCC &C,
FunctionAnalysisManager &FAM =
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
+ bool IsRecursive = C.size() > 1;
for (LazyCallGraph::Node &N : C) {
Function &OldF = N.getFunction();
@@ -920,8 +920,8 @@ PreservedAnalyses ArgumentPromotionPass::run(LazyCallGraph::SCC &C,
};
const TargetTransformInfo &TTI = FAM.getResult<TargetIRAnalysis>(OldF);
- Function *NewF =
- promoteArguments(&OldF, AARGetter, MaxElements, None, TTI);
+ Function *NewF = promoteArguments(&OldF, AARGetter, MaxElements, None,
+ TTI, IsRecursive);
if (!NewF)
continue;
LocalChange = true;
@@ -1022,6 +1022,7 @@ bool ArgPromotion::runOnSCC(CallGraphSCC &SCC) {
do {
LocalChange = false;
// Attempt to promote arguments from all functions in this SCC.
+ bool IsRecursive = SCC.size() > 1;
for (CallGraphNode *OldNode : SCC) {
Function *OldF = OldNode->getFunction();
if (!OldF)
@@ -1038,8 +1039,9 @@ bool ArgPromotion::runOnSCC(CallGraphSCC &SCC) {
const TargetTransformInfo &TTI =
getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*OldF);
- if (Function *NewF = promoteArguments(OldF, AARGetter, MaxElements,
- {ReplaceCallSite}, TTI)) {
+ if (Function *NewF =
+ promoteArguments(OldF, AARGetter, MaxElements, {ReplaceCallSite},
+ TTI, IsRecursive)) {
LocalChange = true;
// Update the call graph for the newly promoted function.
diff --git a/llvm/test/Transforms/ArgumentPromotion/pr42028-recursion.ll b/llvm/test/Transforms/ArgumentPromotion/pr42028-recursion.ll
new file mode 100644
index 0000000000000..6f90573b0565f
--- /dev/null
+++ b/llvm/test/Transforms/ArgumentPromotion/pr42028-recursion.ll
@@ -0,0 +1,54 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
+; RUN: opt -S < %s -argpromotion | FileCheck %s
+
+; This shouldn't get infinitely promoted.
+
+%S = type { %S* }
+
+define i32 @test_inf_promote_caller(i32 %arg) {
+; CHECK-LABEL: define {{[^@]+}}@test_inf_promote_caller
+; CHECK-SAME: (i32 [[ARG:%.*]]) {
+; CHECK-NEXT: bb:
+; CHECK-NEXT: [[TMP:%.*]] = alloca [[S:%.*]], align 8
+; CHECK-NEXT: [[TMP1:%.*]] = alloca [[S]], align 8
+; CHECK-NEXT: [[TMP2:%.*]] = call i32 @test_inf_promote_callee(%S* [[TMP]], %S* [[TMP1]])
+; CHECK-NEXT: ret i32 0
+;
+bb:
+ %tmp = alloca %S
+ %tmp1 = alloca %S
+ %tmp2 = call i32 @test_inf_promote_callee(%S* %tmp, %S* %tmp1)
+ ret i32 0
+}
+
+define internal i32 @test_inf_promote_callee(%S* %arg, %S* %arg1) {
+; CHECK-LABEL: define {{[^@]+}}@test_inf_promote_callee
+; CHECK-SAME: (%S* [[ARG:%.*]], %S* [[ARG1:%.*]]) {
+; CHECK-NEXT: bb:
+; CHECK-NEXT: [[TMP:%.*]] = getelementptr [[S:%.*]], %S* [[ARG1]], i32 0, i32 0
+; CHECK-NEXT: [[TMP2:%.*]] = load %S*, %S** [[TMP]], align 8
+; CHECK-NEXT: [[TMP3:%.*]] = getelementptr [[S]], %S* [[ARG]], i32 0, i32 0
+; CHECK-NEXT: [[TMP4:%.*]] = load %S*, %S** [[TMP3]], align 8
+; CHECK-NEXT: [[TMP5:%.*]] = call i32 @test_inf_promote_callee2(%S* [[TMP4]], %S* [[TMP2]])
+; CHECK-NEXT: ret i32 0
+;
+bb:
+ %tmp = getelementptr %S, %S* %arg1, i32 0, i32 0
+ %tmp2 = load %S*, %S** %tmp
+ %tmp3 = getelementptr %S, %S* %arg, i32 0, i32 0
+ %tmp4 = load %S*, %S** %tmp3
+ %tmp5 = call i32 @test_inf_promote_callee2(%S* %tmp4, %S* %tmp2)
+ ret i32 0
+}
+
+define internal i32 @test_inf_promote_callee2(%S* %arg, %S* %arg1) {
+; CHECK-LABEL: define {{[^@]+}}@test_inf_promote_callee2
+; CHECK-SAME: (%S* [[ARG:%.*]], %S* [[ARG1:%.*]]) {
+; CHECK-NEXT: [[R:%.*]] = call i32 @test_inf_promote_callee(%S* [[ARG]], %S* [[ARG1]])
+; CHECK-NEXT: ret i32 0
+;
+ %r = call i32 @test_inf_promote_callee(%S* %arg, %S* %arg1)
+ ret i32 0
+}
+
+declare i32 @wibble(...)
More information about the llvm-commits
mailing list