Skip to content

Commit 1301ba4

Browse files
Add a check to stop recursive promotion of ptr args
1 parent 677fb7b commit 1301ba4

File tree

1 file changed

+20
-11
lines changed

1 file changed

+20
-11
lines changed

llvm/lib/Transforms/IPO/ArgumentPromotion.cpp

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,6 @@ static bool allCallersPassValidPointerForArgument(Argument *Arg,
446446
/// parts it can be promoted into.
447447
static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
448448
unsigned MaxElements, bool IsRecursive,
449-
bool IsSelfRecursive,
450449
SmallVectorImpl<OffsetAndArgPart> &ArgPartsVec) {
451450
// Quick exit for unused arguments
452451
if (Arg->use_empty())
@@ -613,7 +612,7 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
613612

614613
auto *CB = dyn_cast<CallBase>(V);
615614
Value *PtrArg = dyn_cast<Value>(U);
616-
if (IsSelfRecursive && CB && PtrArg) {
615+
if (IsRecursive && CB && PtrArg) {
617616
Type *PtrTy = PtrArg->getType();
618617
Align PtrAlign = PtrArg->getPointerAlignment(DL);
619618
APInt Offset(DL.getIndexTypeSizeInBits(PtrArg->getType()), 0);
@@ -626,6 +625,23 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
626625
if (Offset.getSignificantBits() >= 64)
627626
return false;
628627

628+
// If this is a recursive function and one of the argument types is a
629+
// pointer that isn't loaded to a non pointer type, it can lead to
630+
// recursive promotion. Look for any Load candidates above the function
631+
// call that load a non pointer type from this argument pointer. If we
632+
// don't find even one such use, return false. For reference, you can
633+
// refer to Transforms/ArgumentPromotion/pr42028-recursion.ll and
634+
// Transforms/ArgumentPromotion/2008-09-08-CGUpdateSelfEdge.ll
635+
// testcases.
636+
bool doesPointerResolve = false;
637+
for (auto Load : Loads)
638+
if (Load->getPointerOperand() == PtrArg &&
639+
!Load->getType()->isPointerTy())
640+
doesPointerResolve = true;
641+
642+
if (!doesPointerResolve)
643+
return false;
644+
629645
int64_t Off = Offset.getSExtValue();
630646
auto Pair = ArgParts.try_emplace(Off, ArgPart{PtrTy, PtrAlign, nullptr});
631647
ArgPart &Part = Pair.first->second;
@@ -748,10 +764,6 @@ static bool areTypesABICompatible(ArrayRef<Type *> Types, const Function &F,
748764
/// calls the DoPromotion method.
749765
static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
750766
unsigned MaxElements, bool IsRecursive) {
751-
// Due to complexity of handling cases where the SCC has more than one
752-
// component. We want to limit argument promotion of recursive calls to
753-
// just functions that directly call themselves.
754-
bool IsSelfRecursive = false;
755767
// Don't perform argument promotion for naked functions; otherwise we can end
756768
// up removing parameters that are seemingly 'not used' as they are referred
757769
// to in the assembly.
@@ -797,10 +809,8 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
797809
if (CB->isMustTailCall())
798810
return nullptr;
799811

800-
if (CB->getFunction() == F) {
812+
if (CB->getFunction() == F)
801813
IsRecursive = true;
802-
IsSelfRecursive = true;
803-
}
804814
}
805815

806816
// Can't change signature of musttail caller
@@ -834,8 +844,7 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
834844
// If we can promote the pointer to its value.
835845
SmallVector<OffsetAndArgPart, 4> ArgParts;
836846

837-
if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, IsSelfRecursive,
838-
ArgParts)) {
847+
if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts)) {
839848
SmallVector<Type *, 4> Types;
840849
for (const auto &Pair : ArgParts)
841850
Types.push_back(Pair.second.Ty);

0 commit comments

Comments
 (0)