Author: baldrick Date: Sun Jan 6 12:27:01 2008 New Revision: 45658 URL: http://llvm.org/viewvc/llvm-project?rev=45658&view=rev Log: The transform that tries to turn calls to bitcast functions into direct calls bails out unless caller and callee have essentially equivalent parameter attributes. This is illogical - the callee's attributes should be of no relevance here. Rework the logic, which incidentally fixes a crash when removed arguments have attributes.
Added: llvm/trunk/test/Transforms/InstCombine/2008-01-06-BitCastAttributes.ll Modified: llvm/trunk/include/llvm/ParameterAttributes.h llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp llvm/trunk/lib/VMCore/ParameterAttributes.cpp llvm/trunk/lib/VMCore/Verifier.cpp Modified: llvm/trunk/include/llvm/ParameterAttributes.h URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ParameterAttributes.h?rev=45658&r1=45657&r2=45658&view=diff ============================================================================== --- llvm/trunk/include/llvm/ParameterAttributes.h (original) +++ llvm/trunk/include/llvm/ParameterAttributes.h Sun Jan 6 12:27:01 2008 @@ -22,6 +22,8 @@ #include <cassert> namespace llvm { +class Type; + namespace ParamAttr { /// Function parameters and results can have attributes to indicate how they @@ -44,13 +46,6 @@ 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; - /// @brief Attributes that only apply to function parameters. const uint16_t ParameterOnly = ByVal | InReg | Nest | StructRet; @@ -63,10 +58,6 @@ /// @brief Attributes that only apply to pointers. const uint16_t PointerTypeOnly = ByVal | Nest | NoAlias | StructRet; -/// @brief Attributes that do not apply to void type function return values. -const uint16_t VoidTypeIncompatible = IntegerTypeOnly | PointerTypeOnly | - ParameterOnly; - /// @brief Attributes that are mutually incompatible. const uint16_t MutuallyIncompatible[3] = { ByVal | InReg | Nest | StructRet, @@ -74,6 +65,9 @@ ReadNone | ReadOnly }; +/// @brief Which of the given attributes do not apply to the type. +uint16_t incompatibleWithType (const Type *Ty, uint16_t attrs); + } // end namespace ParamAttr /// This is just a pair of values to associate a set of parameter attributes @@ -158,11 +152,6 @@ static const ParamAttrsList *excludeAttrs(const ParamAttrsList *PAL, uint16_t idx, uint16_t attrs); - /// 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 /// @{ Modified: llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=45658&r1=45657&r2=45658&view=diff ============================================================================== --- llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp (original) +++ llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp Sun Jan 6 12:27:01 2008 @@ -505,7 +505,7 @@ const Type *RetTy = FTy->getReturnType(); if (DeadRetVal.count(F)) { RetTy = Type::VoidTy; - RAttrs &= ~ParamAttr::VoidTypeIncompatible; + RAttrs &= ~ParamAttr::incompatibleWithType(RetTy, RAttrs); DeadRetVal.erase(F); } @@ -561,8 +561,7 @@ // The call return attributes. uint16_t RAttrs = PAL ? PAL->getParamAttrs(0) : 0; // Adjust in case the function was changed to return void. - if (NF->getReturnType() == Type::VoidTy) - RAttrs &= ~ParamAttr::VoidTypeIncompatible; + RAttrs &= ~ParamAttr::incompatibleWithType(NF->getReturnType(), RAttrs); if (RAttrs) ParamAttrsVec.push_back(ParamAttrsWithIndex::get(0, RAttrs)); Modified: llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp?rev=45658&r1=45657&r2=45658&view=diff ============================================================================== --- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original) +++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Sun Jan 6 12:27:01 2008 @@ -8074,6 +8074,7 @@ return false; Function *Callee = cast<Function>(CE->getOperand(0)); Instruction *Caller = CS.getInstruction(); + const ParamAttrsList* CallerPAL = CS.getParamAttrs(); // Okay, this is a cast from a function to a different type. Unless doing so // would cause a type conversion of one of our arguments, change this call to @@ -8082,13 +8083,6 @@ const FunctionType *FT = Callee->getFunctionType(); const Type *OldRetTy = Caller->getType(); - const ParamAttrsList* CallerPAL = CS.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(CallerPAL, Callee->getParamAttrs())) - return false; - // Check to see if we are changing the return type... if (OldRetTy != FT->getReturnType()) { if (Callee->isDeclaration() && !Caller->use_empty() && @@ -8103,6 +8097,11 @@ FT->getReturnType() != Type::VoidTy) return false; // Cannot transform this return value. + if (!Caller->use_empty() && CallerPAL && + ParamAttr::incompatibleWithType(FT->getReturnType(), + CallerPAL->getParamAttrs(0))) + return false; // Attribute not compatible with transformed value. + // If the callsite is an invoke instruction, and the return value is used by // a PHI node in a successor, we cannot change the return type of the call // because there is no place to put the cast instruction (without breaking @@ -8126,7 +8125,11 @@ const Type *ActTy = (*AI)->getType(); if (!CastInst::isCastable(ActTy, ParamTy)) - return false; + return false; // Cannot transform this parameter value. + + if (CallerPAL && + ParamAttr::incompatibleWithType(ParamTy, CallerPAL->getParamAttrs(i+1))) + return false; // Attribute not compatible with transformed value. ConstantInt *c = dyn_cast<ConstantInt>(*AI); // Some conversions are safe even if we do not have a body. @@ -8144,10 +8147,32 @@ Callee->isDeclaration()) return false; // Do not delete arguments unless we have a function body... + if (FT->getNumParams() < NumActualArgs && FT->isVarArg()) + // In this case we have more arguments than the new function type, but we + // won't be dropping them. Some of them may have attributes. If so, we + // cannot perform the transform because attributes are not allowed after + // the end of the function type. + if (CallerPAL && CallerPAL->size() && + CallerPAL->getParamIndex(CallerPAL->size()-1) > FT->getNumParams()) + return false; + // Okay, we decided that this is a safe thing to do: go ahead and start // inserting cast instructions as necessary... std::vector<Value*> Args; Args.reserve(NumActualArgs); + ParamAttrsVector attrVec; + attrVec.reserve(NumCommonArgs); + + // Get any return attributes. + uint16_t RAttrs = CallerPAL ? CallerPAL->getParamAttrs(0) : 0; + + // If the return value is not being used, the type may not be compatible + // with the existing attributes. Wipe out any problematic attributes. + RAttrs &= ~ParamAttr::incompatibleWithType(FT->getReturnType(), RAttrs); + + // Add the new return attributes. + if (RAttrs) + attrVec.push_back(ParamAttrsWithIndex::get(0, RAttrs)); AI = CS.arg_begin(); for (unsigned i = 0; i != NumCommonArgs; ++i, ++AI) { @@ -8160,6 +8185,11 @@ CastInst *NewCast = CastInst::create(opcode, *AI, ParamTy, "tmp"); Args.push_back(InsertNewInstBefore(NewCast, *Caller)); } + + // Add any parameter attributes. + uint16_t PAttrs = CallerPAL ? CallerPAL->getParamAttrs(i + 1) : 0; + if (PAttrs) + attrVec.push_back(ParamAttrsWithIndex::get(i + 1, PAttrs)); } // If the function takes more arguments than the call was taking, add them @@ -8187,17 +8217,22 @@ Args.push_back(*AI); } } + + // No need to add parameter attributes - we already checked that there + // aren't any. } if (FT->getReturnType() == Type::VoidTy) Caller->setName(""); // Void type should not have a name. + const ParamAttrsList* NewCallerPAL = ParamAttrsList::get(attrVec); + Instruction *NC; if (InvokeInst *II = dyn_cast<InvokeInst>(Caller)) { NC = new InvokeInst(Callee, II->getNormalDest(), II->getUnwindDest(), Args.begin(), Args.end(), Caller->getName(), Caller); cast<InvokeInst>(NC)->setCallingConv(II->getCallingConv()); - cast<InvokeInst>(NC)->setParamAttrs(CallerPAL); + cast<InvokeInst>(NC)->setParamAttrs(NewCallerPAL); } else { NC = new CallInst(Callee, Args.begin(), Args.end(), Caller->getName(), Caller); @@ -8205,7 +8240,7 @@ if (CI->isTailCall()) cast<CallInst>(NC)->setTailCall(); cast<CallInst>(NC)->setCallingConv(CI->getCallingConv()); - cast<CallInst>(NC)->setParamAttrs(CallerPAL); + cast<CallInst>(NC)->setParamAttrs(NewCallerPAL); } // Insert a cast of the return type as necessary. Modified: llvm/trunk/lib/VMCore/ParameterAttributes.cpp URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/ParameterAttributes.cpp?rev=45658&r1=45657&r2=45658&view=diff ============================================================================== --- llvm/trunk/lib/VMCore/ParameterAttributes.cpp (original) +++ llvm/trunk/lib/VMCore/ParameterAttributes.cpp Sun Jan 6 12:27:01 2008 @@ -7,11 +7,12 @@ // //===----------------------------------------------------------------------===// // -// This file implements the ParamAttrsList class. +// This file implements the ParamAttrsList class and ParamAttr utilities. // //===----------------------------------------------------------------------===// #include "llvm/ParameterAttributes.h" +#include "llvm/DerivedTypes.h" #include "llvm/Support/ManagedStatic.h" using namespace llvm; @@ -63,50 +64,6 @@ 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 ParamAttrsVector &Attrs) { for (unsigned i = 0; i < Attrs.size(); ++i) @@ -229,3 +186,19 @@ return getModified(PAL, modVec); } +uint16_t ParamAttr::incompatibleWithType (const Type *Ty, uint16_t attrs) { + uint16_t Incompatible = None; + + if (!Ty->isInteger()) + Incompatible |= IntegerTypeOnly; + + if (!isa<PointerType>(Ty)) + Incompatible |= PointerTypeOnly; + else if (attrs & ParamAttr::ByVal) { + const PointerType *PTy = cast<PointerType>(Ty); + if (!isa<StructType>(PTy->getElementType())) + Incompatible |= ParamAttr::ByVal; + } + + return attrs & Incompatible; +} Modified: llvm/trunk/lib/VMCore/Verifier.cpp URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Verifier.cpp?rev=45658&r1=45657&r2=45658&view=diff ============================================================================== --- llvm/trunk/lib/VMCore/Verifier.cpp (original) +++ llvm/trunk/lib/VMCore/Verifier.cpp Sun Jan 6 12:27:01 2008 @@ -418,22 +418,10 @@ Attrs->getParamAttrsText(MutI) + "are incompatible!", V); } - uint16_t IType = Attr & ParamAttr::IntegerTypeOnly; - Assert1(!IType || FT->getParamType(Idx-1)->isInteger(), - "Attribute " + Attrs->getParamAttrsText(IType) + - "should only apply to Integer type!", V); - - uint16_t PType = Attr & ParamAttr::PointerTypeOnly; - Assert1(!PType || isa<PointerType>(FT->getParamType(Idx-1)), - "Attribute " + Attrs->getParamAttrsText(PType) + - "should only apply to Pointer type!", V); - - if (Attr & ParamAttr::ByVal) { - const PointerType *Ty = - dyn_cast<PointerType>(FT->getParamType(Idx-1)); - Assert1(!Ty || isa<StructType>(Ty->getElementType()), - "Attribute byval should only apply to pointer to structs!", V); - } + uint16_t IType = ParamAttr::incompatibleWithType(FT->getParamType(Idx-1), + Attr); + Assert1(!IType, "Wrong type for attribute " + + Attrs->getParamAttrsText(IType), V); if (Attr & ParamAttr::Nest) { Assert1(!SawNest, "More than one parameter has attribute nest!", V); Added: llvm/trunk/test/Transforms/InstCombine/2008-01-06-BitCastAttributes.ll URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2008-01-06-BitCastAttributes.ll?rev=45658&view=auto ============================================================================== --- llvm/trunk/test/Transforms/InstCombine/2008-01-06-BitCastAttributes.ll (added) +++ llvm/trunk/test/Transforms/InstCombine/2008-01-06-BitCastAttributes.ll Sun Jan 6 12:27:01 2008 @@ -0,0 +1,23 @@ +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep bitcast | count 2 + +define void @a() { + ret void +} + +define i32 @b(i32* inreg %x) signext { + ret i32 0 +} + +define void @c(...) { + ret void +} + +define void @g(i32* %y) { + call void bitcast (void ()* @a to void (i32*)*)( i32* noalias %y ) + call <2 x i32> bitcast (i32 (i32*)* @b to <2 x i32> (i32*)*)( i32* inreg null ) ; <<2 x i32>>:1 [#uses=0] + %x = call i64 bitcast (i32 (i32*)* @b to i64 (i32)*)( i32 0 ) ; <i64> [#uses=0] + call void bitcast (void (...)* @c to void (i32)*)( i32 0 ) + call i32 bitcast (i32 (i32*)* @b to i32 (i32)*)( i32 zeroext 0 ) ; <i32>:2 [#uses=0] + call void bitcast (void (...)* @c to void (i32)*)( i32 zeroext 0 ) + ret void +} _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits