From e5f8de8639e6f006fca096084c1322d332f58850 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 16 Jan 2011 08:09:24 +0000 Subject: [PATCH] fix PR8932, a case where arg promotion could infinitely promote. llvm-svn: 123574 --- llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 75 +++++++++++++------ .../Transforms/ArgumentPromotion/crash.ll | 21 ++++++ 2 files changed, 72 insertions(+), 24 deletions(-) diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp index 17ef703ad36e..0c650cfe6440 100644 --- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp +++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp @@ -135,47 +135,74 @@ CallGraphNode *ArgPromotion::PromoteArguments(CallGraphNode *CGN) { if (PointerArgs.empty()) return 0; // Second check: make sure that all callers are direct callers. We can't - // transform functions that have indirect callers. - if (F->hasAddressTaken()) - return 0; - + // transform functions that have indirect callers. Also see if the function + // is self-recursive. + bool isSelfRecursive = false; + for (Value::use_iterator UI = F->use_begin(), E = F->use_end(); + UI != E; ++UI) { + CallSite CS(*UI); + // Must be a direct call. + if (CS.getInstruction() == 0 || !CS.isCallee(UI)) return 0; + + if (CS.getInstruction()->getParent()->getParent() == F) + isSelfRecursive = true; + } + // Check to see which arguments are promotable. If an argument is promotable, // add it to ArgsToPromote. SmallPtrSet ArgsToPromote; SmallPtrSet ByValArgsToTransform; for (unsigned i = 0; i != PointerArgs.size(); ++i) { bool isByVal = F->paramHasAttr(PointerArgs[i].second+1, Attribute::ByVal); + Argument *PtrArg = PointerArgs[i].first; + const Type *AgTy = cast(PtrArg->getType())->getElementType(); // If this is a byval argument, and if the aggregate type is small, just // pass the elements, which is always safe. - Argument *PtrArg = PointerArgs[i].first; if (isByVal) { - const Type *AgTy = cast(PtrArg->getType())->getElementType(); if (const StructType *STy = dyn_cast(AgTy)) { if (maxElements > 0 && STy->getNumElements() > maxElements) { DEBUG(dbgs() << "argpromotion disable promoting argument '" << PtrArg->getName() << "' because it would require adding more" << " than " << maxElements << " arguments to the function.\n"); - } else { - // If all the elements are single-value types, we can promote it. - bool AllSimple = true; - for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) - if (!STy->getElementType(i)->isSingleValueType()) { - AllSimple = false; - break; - } - - // Safe to transform, don't even bother trying to "promote" it. - // Passing the elements as a scalar will allow scalarrepl to hack on - // the new alloca we introduce. - if (AllSimple) { - ByValArgsToTransform.insert(PtrArg); - continue; + continue; + } + + // If all the elements are single-value types, we can promote it. + bool AllSimple = true; + for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) { + if (!STy->getElementType(i)->isSingleValueType()) { + AllSimple = false; + break; } } + + // Safe to transform, don't even bother trying to "promote" it. + // Passing the elements as a scalar will allow scalarrepl to hack on + // the new alloca we introduce. + if (AllSimple) { + ByValArgsToTransform.insert(PtrArg); + continue; + } } } + // If the argument is a recursive type and we're in a recursive + // function, we could end up infinitely peeling the function argument. + if (isSelfRecursive) { + if (const StructType *STy = dyn_cast(AgTy)) { + bool RecursiveType = false; + for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) { + if (STy->getElementType(i) == PtrArg->getType()) { + RecursiveType = true; + break; + } + } + if (RecursiveType) + continue; + } + } + // Otherwise, see if we can promote the pointer to its value. if (isSafeToPromoteArgument(PtrArg, isByVal)) ArgsToPromote.insert(PtrArg); @@ -188,9 +215,9 @@ CallGraphNode *ArgPromotion::PromoteArguments(CallGraphNode *CGN) { return DoPromotion(F, ArgsToPromote, ByValArgsToTransform); } -/// AllCalleesPassInValidPointerForArgument - Return true if we can prove that +/// AllCallersPassInValidPointerForArgument - Return true if we can prove that /// all callees pass in a valid pointer for the specified function argument. -static bool AllCalleesPassInValidPointerForArgument(Argument *Arg) { +static bool AllCallersPassInValidPointerForArgument(Argument *Arg) { Function *Callee = Arg->getParent(); unsigned ArgNo = std::distance(Callee->arg_begin(), @@ -310,7 +337,7 @@ bool ArgPromotion::isSafeToPromoteArgument(Argument *Arg, bool isByVal) const { GEPIndicesSet ToPromote; // If the pointer is always valid, any load with first index 0 is valid. - if (isByVal || AllCalleesPassInValidPointerForArgument(Arg)) + if (isByVal || AllCallersPassInValidPointerForArgument(Arg)) SafeToUnconditionallyLoad.insert(IndicesVector(1, 0)); // First, iterate the entry block and mark loads of (geps of) arguments as diff --git a/llvm/test/Transforms/ArgumentPromotion/crash.ll b/llvm/test/Transforms/ArgumentPromotion/crash.ll index e2d3d4de9edb..fed002aa98a9 100644 --- a/llvm/test/Transforms/ArgumentPromotion/crash.ll +++ b/llvm/test/Transforms/ArgumentPromotion/crash.ll @@ -36,3 +36,24 @@ entry: ret i1 undef } + +; PR8932 - infinite promotion. +%0 = type { %0* } + +define i32 @test2(i32 %a) { +init: + %0 = alloca %0 + %1 = alloca %0 + %2 = call i32 @"clay_assign(Chain, Chain)"(%0* %0, %0* %1) + ret i32 0 +} + +define internal i32 @"clay_assign(Chain, Chain)"(%0* %c, %0* %d) { +init: + %0 = getelementptr %0* %d, i32 0, i32 0 + %1 = load %0** %0 + %2 = getelementptr %0* %c, i32 0, i32 0 + %3 = load %0** %2 + %4 = call i32 @"clay_assign(Chain, Chain)"(%0* %3, %0* %1) + ret i32 0 +}