From b0c9b93bb4c591b1cec0491eec34f792cd3cf111 Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Mon, 14 Jan 2008 19:52:09 +0000 Subject: [PATCH] I noticed that the trampoline straightening transformation could drop attributes on varargs call arguments. Also, it could generate invalid IR if the transformed call already had the 'nest' attribute somewhere (this can never happen for code coming from llvm-gcc, but it's a theoretical possibility). Fix both problems. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@45973 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ParameterAttributes.h | 8 ++- .../Scalar/InstructionCombining.cpp | 49 +++++++++++-------- lib/VMCore/ParameterAttributes.cpp | 7 +++ .../InstCombine/2008-01-14-DoubleNest.ll | 24 +++++++++ .../2008-01-14-VarArgTrampoline.ll | 24 +++++++++ 5 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 test/Transforms/InstCombine/2008-01-14-DoubleNest.ll create mode 100644 test/Transforms/InstCombine/2008-01-14-VarArgTrampoline.ll diff --git a/include/llvm/ParameterAttributes.h b/include/llvm/ParameterAttributes.h index ded847e146a..31f9eb4a080 100644 --- a/include/llvm/ParameterAttributes.h +++ b/include/llvm/ParameterAttributes.h @@ -53,7 +53,7 @@ const uint16_t ParameterOnly = ByVal | InReg | Nest | StructRet; const uint16_t ReturnOnly = NoReturn | NoUnwind | ReadNone | ReadOnly; /// @brief Parameter attributes that do not apply to vararg call arguments. -const uint16_t VarArgsIncompatible = Nest | StructRet; +const uint16_t VarArgsIncompatible = StructRet; /// @brief Attributes that are mutually incompatible. const uint16_t MutuallyIncompatible[3] = { @@ -171,6 +171,12 @@ class ParamAttrsList : public FoldingSetNode { return getParamAttrs(i) & attr; } + /// This returns whether the given attribute is set for at least one + /// parameter or for the return value. + /// @returns true if the parameter attribute is set somewhere + /// @brief Determine if a ParameterAttributes is set somewhere + bool hasAttrSomewhere(ParameterAttributes attr) const; + /// The set of ParameterAttributes set in Attributes is converted to a /// string of equivalent mnemonics. This is, presumably, for writing out /// the mnemonics for the assembly writer. diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp index 63919a8d32c..2ad592a8f11 100644 --- a/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/lib/Transforms/Scalar/InstructionCombining.cpp @@ -8385,6 +8385,12 @@ Instruction *InstCombiner::transformCallThroughTrampoline(CallSite CS) { Value *Callee = CS.getCalledValue(); const PointerType *PTy = cast(Callee->getType()); const FunctionType *FTy = cast(PTy->getElementType()); + const ParamAttrsList *Attrs = CS.getParamAttrs(); + + // If the call already has the 'nest' attribute somewhere then give up - + // otherwise 'nest' would occur twice after splicing in the chain. + if (Attrs && Attrs->hasAttrSomewhere(ParamAttr::Nest)) + return 0; IntrinsicInst *Tramp = cast(cast(Callee)->getOperand(0)); @@ -8414,25 +8420,39 @@ Instruction *InstCombiner::transformCallThroughTrampoline(CallSite CS) { std::vector NewArgs; NewArgs.reserve(unsigned(CS.arg_end()-CS.arg_begin())+1); + ParamAttrsVector NewAttrs; + NewAttrs.reserve(Attrs ? Attrs->size() + 1 : 1); + // Insert the nest argument into the call argument list, which may - // mean appending it. + // mean appending it. Likewise for attributes. + + // Add any function result attributes. + uint16_t Attr = Attrs ? Attrs->getParamAttrs(0) : 0; + if (Attr) + NewAttrs.push_back (ParamAttrsWithIndex::get(0, Attr)); + { unsigned Idx = 1; CallSite::arg_iterator I = CS.arg_begin(), E = CS.arg_end(); do { if (Idx == NestIdx) { - // Add the chain argument. + // Add the chain argument and attributes. Value *NestVal = Tramp->getOperand(3); if (NestVal->getType() != NestTy) NestVal = new BitCastInst(NestVal, NestTy, "nest", Caller); NewArgs.push_back(NestVal); + NewAttrs.push_back(ParamAttrsWithIndex::get(NestIdx, NestAttr)); } if (I == E) break; - // Add the original argument. + // Add the original argument and attributes. NewArgs.push_back(*I); + Attr = Attrs ? Attrs->getParamAttrs(Idx) : 0; + if (Attr) + NewAttrs.push_back + (ParamAttrsWithIndex::get(Idx + (Idx >= NestIdx), Attr)); ++Idx, ++I; } while (1); @@ -8440,41 +8460,28 @@ Instruction *InstCombiner::transformCallThroughTrampoline(CallSite CS) { // The trampoline may have been bitcast to a bogus type (FTy). // Handle this by synthesizing a new function type, equal to FTy - // with the chain parameter inserted. Likewise for attributes. + // with the chain parameter inserted. - const ParamAttrsList *Attrs = CS.getParamAttrs(); std::vector NewTypes; - ParamAttrsVector NewAttrs; NewTypes.reserve(FTy->getNumParams()+1); - // Add any function result attributes. - uint16_t Attr = Attrs ? Attrs->getParamAttrs(0) : 0; - if (Attr) - NewAttrs.push_back (ParamAttrsWithIndex::get(0, Attr)); - // Insert the chain's type into the list of parameter types, which may - // mean appending it. Likewise for the chain's attributes. + // mean appending it. { unsigned Idx = 1; FunctionType::param_iterator I = FTy->param_begin(), E = FTy->param_end(); do { - if (Idx == NestIdx) { - // Add the chain's type and attributes. + if (Idx == NestIdx) + // Add the chain's type. NewTypes.push_back(NestTy); - NewAttrs.push_back(ParamAttrsWithIndex::get(NestIdx, NestAttr)); - } if (I == E) break; - // Add the original type and attributes. + // Add the original type. NewTypes.push_back(*I); - Attr = Attrs ? Attrs->getParamAttrs(Idx) : 0; - if (Attr) - NewAttrs.push_back - (ParamAttrsWithIndex::get(Idx + (Idx >= NestIdx), Attr)); ++Idx, ++I; } while (1); diff --git a/lib/VMCore/ParameterAttributes.cpp b/lib/VMCore/ParameterAttributes.cpp index b48e0cecde3..4b63cfb2da7 100644 --- a/lib/VMCore/ParameterAttributes.cpp +++ b/lib/VMCore/ParameterAttributes.cpp @@ -36,6 +36,13 @@ ParamAttrsList::getParamAttrs(uint16_t Index) const { return ParamAttr::None; } +bool ParamAttrsList::hasAttrSomewhere(ParameterAttributes attr) const { + for (unsigned i = 0, e = attrs.size(); i < e; ++i) + if (attrs[i].attrs & attr) + return true; + return false; +} + std::string ParamAttrsList::getParamAttrsText(uint16_t Attrs) { std::string Result; diff --git a/test/Transforms/InstCombine/2008-01-14-DoubleNest.ll b/test/Transforms/InstCombine/2008-01-14-DoubleNest.ll new file mode 100644 index 00000000000..5381db2acbc --- /dev/null +++ b/test/Transforms/InstCombine/2008-01-14-DoubleNest.ll @@ -0,0 +1,24 @@ +; RUN: llvm-as < %s | opt -instcombine -disable-output + + %struct.FRAME.nest = type { i32, i32 (i32*)* } + %struct.__builtin_trampoline = type { [10 x i8] } + +declare i8* @llvm.init.trampoline(i8*, i8*, i8*) nounwind + +declare i32 @f(%struct.FRAME.nest* nest , i32*) + +define i32 @nest(i32 %n) { +entry: + %FRAME.0 = alloca %struct.FRAME.nest, align 8 ; <%struct.FRAME.nest*> [#uses=3] + %TRAMP.216 = alloca [10 x i8], align 16 ; <[10 x i8]*> [#uses=1] + %TRAMP.216.sub = getelementptr [10 x i8]* %TRAMP.216, i32 0, i32 0 ; [#uses=1] + %tmp3 = getelementptr %struct.FRAME.nest* %FRAME.0, i32 0, i32 0 ; [#uses=1] + store i32 %n, i32* %tmp3, align 8 + %FRAME.06 = bitcast %struct.FRAME.nest* %FRAME.0 to i8* ; [#uses=1] + %tramp = call i8* @llvm.init.trampoline( i8* %TRAMP.216.sub, i8* bitcast (i32 (%struct.FRAME.nest*, i32*)* @f to i8*), i8* %FRAME.06 ) ; [#uses=1] + %tmp7 = getelementptr %struct.FRAME.nest* %FRAME.0, i32 0, i32 1 ; [#uses=1] + %tmp89 = bitcast i8* %tramp to i32 (i32*)* ; [#uses=2] + store i32 (i32*)* %tmp89, i32 (i32*)** %tmp7, align 8 + %tmp2.i = call i32 %tmp89( i32* nest null ) ; [#uses=1] + ret i32 %tmp2.i +} diff --git a/test/Transforms/InstCombine/2008-01-14-VarArgTrampoline.ll b/test/Transforms/InstCombine/2008-01-14-VarArgTrampoline.ll new file mode 100644 index 00000000000..e35794a813b --- /dev/null +++ b/test/Transforms/InstCombine/2008-01-14-VarArgTrampoline.ll @@ -0,0 +1,24 @@ +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep zeroext + + %struct.FRAME.nest = type { i32, i32 (...)* } + %struct.__builtin_trampoline = type { [10 x i8] } + +declare i8* @llvm.init.trampoline(i8*, i8*, i8*) nounwind + +declare i32 @f(%struct.FRAME.nest* nest , ...) + +define i32 @nest(i32 %n) { +entry: + %FRAME.0 = alloca %struct.FRAME.nest, align 8 ; <%struct.FRAME.nest*> [#uses=3] + %TRAMP.216 = alloca [10 x i8], align 16 ; <[10 x i8]*> [#uses=1] + %TRAMP.216.sub = getelementptr [10 x i8]* %TRAMP.216, i32 0, i32 0 ; [#uses=1] + %tmp3 = getelementptr %struct.FRAME.nest* %FRAME.0, i32 0, i32 0 ; [#uses=1] + store i32 %n, i32* %tmp3, align 8 + %FRAME.06 = bitcast %struct.FRAME.nest* %FRAME.0 to i8* ; [#uses=1] + %tramp = call i8* @llvm.init.trampoline( i8* %TRAMP.216.sub, i8* bitcast (i32 (%struct.FRAME.nest*, ...)* @f to i8*), i8* %FRAME.06 ) ; [#uses=1] + %tmp7 = getelementptr %struct.FRAME.nest* %FRAME.0, i32 0, i32 1 ; [#uses=1] + %tmp89 = bitcast i8* %tramp to i32 (...)* ; [#uses=2] + store i32 (...)* %tmp89, i32 (...)** %tmp7, align 8 + %tmp2.i = call i32 (...)* %tmp89( i32 zeroext 0 ) ; [#uses=1] + ret i32 %tmp2.i +}