diff --git a/include/llvm/ParameterAttributes.h b/include/llvm/ParameterAttributes.h index 4c48d8707fb..1dd98eaa901 100644 --- a/include/llvm/ParameterAttributes.h +++ b/include/llvm/ParameterAttributes.h @@ -43,6 +43,34 @@ enum Attributes { ReadOnly = 1 << 10 ///< Function only reads from memory }; +/// These attributes can safely be dropped from a function or a function call: +/// doing so may reduce the number of optimizations performed, but it will not +/// change a correct program into an incorrect one. +/// @brief Attributes that do not change the calling convention. +const uint16_t Informative = NoReturn | NoUnwind | NoAlias | + ReadNone | ReadOnly; + +/// The following attribute sets are used by the verifier: + +/// @brief Attributes that only apply to function parameters. +const uint16_t ParameterOnly = ByVal | InReg | Nest | StructRet; + +/// @brief Attributes that only apply to function return values. +const uint16_t ReturnOnly = NoReturn | NoUnwind | ReadNone | ReadOnly; + +/// @brief Attributes that only apply to integers. +const uint16_t IntegerTypeOnly = SExt | ZExt; + +/// @brief Attributes that only apply to pointers. +const uint16_t PointerTypeOnly = ByVal | Nest | NoAlias | StructRet; + +/// @brief Attributes that are mutually incompatible. +const uint16_t MutuallyIncompatible[3] = { + ByVal | InReg | Nest | StructRet, + ZExt | SExt, + ReadNone | ReadOnly +}; + } /// This is just a pair of values to associate a set of parameter attributes @@ -110,6 +138,11 @@ class ParamAttrsList : public FoldingSetNode { /// @brief Get a ParamAttrsList instance. static ParamAttrsList *get(const ParamAttrsVector &attrVec); + /// Returns whether each of the specified lists of attributes can be safely + /// replaced with the other in a function or a function call. + /// @brief Whether one attribute list can safely replace the other. + static bool areCompatible(const ParamAttrsList *A, const ParamAttrsList *B); + /// @} /// @name Accessors /// @{ diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp index 5537631eef9..85fd2c3bdb7 100644 --- a/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/lib/Transforms/Scalar/InstructionCombining.cpp @@ -7990,11 +7990,12 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) { const FunctionType *ActualFT = cast(cast(CE->getType())->getElementType()); - // If the parameter attributes don't match up, don't do the xform. We don't - // want to lose an sret attribute or something. - if (FT->getParamAttrs() != ActualFT->getParamAttrs()) + // If the parameter attributes are not compatible, don't do the xform. We + // don't want to lose an sret attribute or something. + if (!ParamAttrsList::areCompatible(FT->getParamAttrs(), + ActualFT->getParamAttrs())) return false; - + // Check to see if we are changing the return type... if (OldRetTy != FT->getReturnType()) { if (Callee->isDeclaration() && !Caller->use_empty() && diff --git a/lib/VMCore/Function.cpp b/lib/VMCore/Function.cpp index 6c293718626..2b83e6b9172 100644 --- a/lib/VMCore/Function.cpp +++ b/lib/VMCore/Function.cpp @@ -86,7 +86,6 @@ ParamAttrsList::getParamAttrs(uint16_t Index) const { return ParamAttr::None; } - std::string ParamAttrsList::getParamAttrsText(uint16_t Attrs) { std::string Result; @@ -115,6 +114,50 @@ ParamAttrsList::getParamAttrsText(uint16_t Attrs) { return Result; } +/// onlyInformative - Returns whether only informative attributes are set. +static inline bool onlyInformative(uint16_t attrs) { + return !(attrs & ~ParamAttr::Informative); +} + +bool +ParamAttrsList::areCompatible(const ParamAttrsList *A, const ParamAttrsList *B){ + if (A == B) + return true; + unsigned ASize = A ? A->size() : 0; + unsigned BSize = B ? B->size() : 0; + unsigned AIndex = 0; + unsigned BIndex = 0; + + while (AIndex < ASize && BIndex < BSize) { + uint16_t AIdx = A->getParamIndex(AIndex); + uint16_t BIdx = B->getParamIndex(BIndex); + uint16_t AAttrs = A->getParamAttrsAtIndex(AIndex); + uint16_t BAttrs = B->getParamAttrsAtIndex(AIndex); + + if (AIdx < BIdx) { + if (!onlyInformative(AAttrs)) + return false; + ++AIndex; + } else if (BIdx < AIdx) { + if (!onlyInformative(BAttrs)) + return false; + ++BIndex; + } else { + if (!onlyInformative(AAttrs ^ BAttrs)) + return false; + ++AIndex; + ++BIndex; + } + } + for (; AIndex < ASize; ++AIndex) + if (!onlyInformative(A->getParamAttrsAtIndex(AIndex))) + return false; + for (; BIndex < BSize; ++BIndex) + if (!onlyInformative(B->getParamAttrsAtIndex(AIndex))) + return false; + return true; +} + void ParamAttrsList::Profile(FoldingSetNodeID &ID) const { for (unsigned i = 0; i < attrs.size(); ++i) { diff --git a/lib/VMCore/Verifier.cpp b/lib/VMCore/Verifier.cpp index 1f726afa5d7..a547721d68d 100644 --- a/lib/VMCore/Verifier.cpp +++ b/lib/VMCore/Verifier.cpp @@ -393,30 +393,6 @@ void Verifier::visitFunction(Function &F) { Assert1(!FT->isStructReturn() || FT->getReturnType() == Type::VoidTy, "Invalid struct-return function!", &F); - const uint16_t ReturnIncompatible = - ParamAttr::ByVal | ParamAttr::InReg | - ParamAttr::Nest | ParamAttr::StructRet; - - const uint16_t ParameterIncompatible = - ParamAttr::NoReturn | ParamAttr::NoUnwind | - ParamAttr::ReadNone | ParamAttr::ReadOnly; - - const uint16_t MutuallyIncompatible[3] = { - ParamAttr::ByVal | ParamAttr::InReg | - ParamAttr::Nest | ParamAttr::StructRet, - - ParamAttr::ZExt | ParamAttr::SExt, - - ParamAttr::ReadNone | ParamAttr::ReadOnly - }; - - const uint16_t IntegerTypeOnly = - ParamAttr::SExt | ParamAttr::ZExt; - - const uint16_t PointerTypeOnly = - ParamAttr::ByVal | ParamAttr::Nest | - ParamAttr::NoAlias | ParamAttr::StructRet; - bool SawSRet = false; if (const ParamAttrsList *Attrs = FT->getParamAttrs()) { @@ -426,28 +402,29 @@ void Verifier::visitFunction(Function &F) { uint16_t Attr = Attrs->getParamAttrs(Idx); if (!Idx) { - uint16_t RetI = Attr & ReturnIncompatible; + uint16_t RetI = Attr & ParamAttr::ParameterOnly; Assert1(!RetI, "Attribute " + Attrs->getParamAttrsText(RetI) + "should not apply to functions!", &F); } else { - uint16_t ParmI = Attr & ParameterIncompatible; + uint16_t ParmI = Attr & ParamAttr::ReturnOnly; Assert1(!ParmI, "Attribute " + Attrs->getParamAttrsText(ParmI) + "should only be applied to function!", &F); } - for (unsigned i = 0; i < array_lengthof(MutuallyIncompatible); ++i) { - uint16_t MutI = Attr & MutuallyIncompatible[i]; + for (unsigned i = 0; + i < array_lengthof(ParamAttr::MutuallyIncompatible); ++i) { + uint16_t MutI = Attr & ParamAttr::MutuallyIncompatible[i]; Assert1(!(MutI & (MutI - 1)), "Attributes " + Attrs->getParamAttrsText(MutI) + "are incompatible!", &F); } - uint16_t IType = Attr & IntegerTypeOnly; + uint16_t IType = Attr & ParamAttr::IntegerTypeOnly; Assert1(!IType || FT->getParamType(Idx-1)->isInteger(), "Attribute " + Attrs->getParamAttrsText(IType) + "should only apply to Integer type!", &F); - uint16_t PType = Attr & PointerTypeOnly; + uint16_t PType = Attr & ParamAttr::PointerTypeOnly; Assert1(!PType || isa(FT->getParamType(Idx-1)), "Attribute " + Attrs->getParamAttrsText(PType) + "should only apply to Pointer type!", &F); diff --git a/test/Transforms/InstCombine/2007-11-25-CompatibleAttributes.ll b/test/Transforms/InstCombine/2007-11-25-CompatibleAttributes.ll new file mode 100644 index 00000000000..60a4b3b4ab5 --- /dev/null +++ b/test/Transforms/InstCombine/2007-11-25-CompatibleAttributes.ll @@ -0,0 +1,12 @@ +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | not grep bitcast +; PR1716 + +@.str = internal constant [4 x i8] c"%d\0A\00" ; <[4 x i8]*> [#uses=1] + +define i32 @main(i32 %argc, i8** %argv) { +entry: + %tmp32 = tail call i32 (i8* noalias , ...) nounwind * bitcast (i32 (i8*, ...) nounwind * @printf to i32 (i8* noalias , ...) nounwind *)( i8* getelementptr ([4 x i8]* @.str, i32 0, i32 0) noalias , i32 0 ) nounwind ; [#uses=0] + ret i32 undef +} + +declare i32 @printf(i8*, ...) nounwind