maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Nikita Popov 2022-02-10 10:38:37 +01:00
parent a3655de2c8
commit e24067819f
2 changed files with 68 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
View File

@ -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.

54
llvm/test/Transforms/ArgumentPromotion/pr42028-recursion.ll Normal file
View File

@ -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(...)