From 3f4385a0b6f52ff0feb99ddb6431b7e7960b13af Mon Sep 17 00:00:00 2001 From: Stephen Lin Date: Sun, 30 Jun 2013 20:26:21 +0000 Subject: [PATCH] DeadArgumentElimination: keep return value on functions that have a live argument with the 'returned' attribute (rather than generate invalid IR); however, if both can be eliminated, both will be git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@185290 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/IPO/DeadArgumentElimination.cpp | 88 ++++++++++++++++++-------- test/Transforms/DeadArgElim/returned.ll | 55 ++++++++++++++++ 2 files changed, 115 insertions(+), 28 deletions(-) create mode 100644 test/Transforms/DeadArgElim/returned.ll diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp index 2e27ae04170..6ee6162ac3c 100644 --- a/lib/Transforms/IPO/DeadArgumentElimination.cpp +++ b/lib/Transforms/IPO/DeadArgumentElimination.cpp @@ -716,10 +716,42 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) { FunctionType *FTy = F->getFunctionType(); std::vector Params; + // Keep track of if we have a live 'returned' argument + bool HasLiveReturnedArg = false; + // Set up to build a new list of parameter attributes. SmallVector AttributesVec; const AttributeSet &PAL = F->getAttributes(); + // Remember which arguments are still alive. + SmallVector ArgAlive(FTy->getNumParams(), false); + // Construct the new parameter list from non-dead arguments. Also construct + // a new set of parameter attributes to correspond. Skip the first parameter + // attribute, since that belongs to the return value. + unsigned i = 0; + for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end(); + I != E; ++I, ++i) { + RetOrArg Arg = CreateArg(F, i); + if (LiveValues.erase(Arg)) { + Params.push_back(I->getType()); + ArgAlive[i] = true; + + // Get the original parameter attributes (skipping the first one, that is + // for the return value. + if (PAL.hasAttributes(i + 1)) { + AttrBuilder B(PAL, i + 1); + if (B.contains(Attribute::Returned)) + HasLiveReturnedArg = true; + AttributesVec. + push_back(AttributeSet::get(F->getContext(), Params.size(), B)); + } + } else { + ++NumArgumentsEliminated; + DEBUG(dbgs() << "DAE - Removing argument " << i << " (" << I->getName() + << ") from " << F->getName() << "\n"); + } + } + // Find out the new return value. Type *RetTy = FTy->getReturnType(); Type *NRetTy = NULL; @@ -728,7 +760,27 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) { // -1 means unused, other numbers are the new index SmallVector NewRetIdxs(RetCount, -1); std::vector RetTypes; - if (RetTy->isVoidTy()) { + + // If there is a function with a live 'returned' argument but a dead return + // value, then there are two possible actions: + // 1) Eliminate the return value and take off the 'returned' attribute on the + // argument. + // 2) Retain the 'returned' attribute and treat the return value (but not the + // entire function) as live so that it is not eliminated. + // + // It's not clear in the general case which option is more profitable because, + // even in the absence of explicit uses of the return value, code generation + // is free to use the 'returned' attribute to do things like eliding + // save/restores of registers across calls. Whether or not this happens is + // target and ABI-specific as well as depending on the amount of register + // pressure, so there's no good way for an IR-level pass to figure this out. + // + // Fortunately, the only places where 'returned' is currently generated by + // the FE are places where 'returned' is basically free and almost always a + // performance win, so the second option can just be used always for now. + // + // This should be revisited if 'returned' is ever applied more liberally. + if (RetTy->isVoidTy() || HasLiveReturnedArg) { NRetTy = RetTy; } else { StructType *STy = dyn_cast(RetTy); @@ -798,33 +850,6 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) { if (RAttrs.hasAttributes(AttributeSet::ReturnIndex)) AttributesVec.push_back(AttributeSet::get(NRetTy->getContext(), RAttrs)); - // Remember which arguments are still alive. - SmallVector ArgAlive(FTy->getNumParams(), false); - // Construct the new parameter list from non-dead arguments. Also construct - // a new set of parameter attributes to correspond. Skip the first parameter - // attribute, since that belongs to the return value. - unsigned i = 0; - for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end(); - I != E; ++I, ++i) { - RetOrArg Arg = CreateArg(F, i); - if (LiveValues.erase(Arg)) { - Params.push_back(I->getType()); - ArgAlive[i] = true; - - // Get the original parameter attributes (skipping the first one, that is - // for the return value. - if (PAL.hasAttributes(i + 1)) { - AttrBuilder B(PAL, i + 1); - AttributesVec. - push_back(AttributeSet::get(F->getContext(), Params.size(), B)); - } - } else { - ++NumArgumentsEliminated; - DEBUG(dbgs() << "DAE - Removing argument " << i << " (" << I->getName() - << ") from " << F->getName() << "\n"); - } - } - if (PAL.hasAttributes(AttributeSet::FunctionIndex)) AttributesVec.push_back(AttributeSet::get(F->getContext(), PAL.getFnAttributes())); @@ -885,6 +910,13 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) { // Get original parameter attributes, but skip return attributes. if (CallPAL.hasAttributes(i + 1)) { AttrBuilder B(CallPAL, i + 1); + // If the return type has changed, then get rid of 'returned' on the + // call site. The alternative is to make all 'returned' attributes on + // call sites keep the return value alive just like 'returned' + // attributes on function declaration but it's less clearly a win + // and this is not an expected case anyway + if (NRetTy != RetTy && B.contains(Attribute::Returned)) + B.removeAttribute(Attribute::Returned); AttributesVec. push_back(AttributeSet::get(F->getContext(), Args.size(), B)); } diff --git a/test/Transforms/DeadArgElim/returned.ll b/test/Transforms/DeadArgElim/returned.ll new file mode 100644 index 00000000000..c4d3ec7d3c6 --- /dev/null +++ b/test/Transforms/DeadArgElim/returned.ll @@ -0,0 +1,55 @@ +; RUN: opt < %s -deadargelim -S | FileCheck %s + +%Ty = type { i32, i32 } + +; sanity check that the argument and return value are both dead +; CHECK: define internal void @test1() + +define internal %Ty* @test1(%Ty* %this) { + ret %Ty* %this +} + +; do not keep alive the return value of a function with a dead 'returned' argument +; CHECK: define internal void @test2() + +define internal %Ty* @test2(%Ty* returned %this) { + ret %Ty* %this +} + +; dummy to keep 'this' alive +@dummy = global %Ty* null + +; sanity check that return value is dead +; CHECK: define internal void @test3(%Ty* %this) + +define internal %Ty* @test3(%Ty* %this) { + store volatile %Ty* %this, %Ty** @dummy + ret %Ty* %this +} + +; keep alive return value of a function if the 'returned' argument is live +; CHECK: define internal %Ty* @test4(%Ty* returned %this) + +define internal %Ty* @test4(%Ty* returned %this) { + store volatile %Ty* %this, %Ty** @dummy + ret %Ty* %this +} + +; don't do this if 'returned' is on the call site... +; CHECK: define internal void @test5(%Ty* %this) + +define internal %Ty* @test5(%Ty* %this) { + store volatile %Ty* %this, %Ty** @dummy + ret %Ty* %this +} + +define %Ty* @caller(%Ty* %this) { + %1 = call %Ty* @test1(%Ty* %this) + %2 = call %Ty* @test2(%Ty* %this) + %3 = call %Ty* @test3(%Ty* %this) + %4 = call %Ty* @test4(%Ty* %this) +; ...instead, drop 'returned' form the call site +; CHECK: call void @test5(%Ty* %this) + %5 = call %Ty* @test5(%Ty* returned %this) + ret %Ty* %this +} -- 2.11.0