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.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@45658 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/ParameterAttributes.h

@@ -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 @@ 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;
-
 /// @brief Attributes that only apply to function parameters.
 const uint16_t ParameterOnly = ByVal | InReg | Nest | StructRet;
 
@@ -63,10 +58,6 @@ const uint16_t IntegerTypeOnly = SExt | ZExt;
 /// @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 @@ const uint16_t MutuallyIncompatible[3] = {
   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 @@ class ParamAttrsList : public FoldingSetNode {
     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
   /// @{

lib/Transforms/IPO/DeadArgumentElimination.cpp

@@ -505,7 +505,7 @@ void DAE::RemoveDeadArgumentsFromFunction(Function *F) {
   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 @@ void DAE::RemoveDeadArgumentsFromFunction(Function *F) {
     // 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));
 

lib/Transforms/Scalar/InstructionCombining.cpp

@@ -8074,6 +8074,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
   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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
         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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
       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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
       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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
           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 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     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.

lib/VMCore/ParameterAttributes.cpp

@@ -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 @@ 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 ParamAttrsVector &Attrs) {
   for (unsigned i = 0; i < Attrs.size(); ++i)
@@ -229,3 +186,19 @@ ParamAttrsList::excludeAttrs(const ParamAttrsList *PAL,
   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;
+}

lib/VMCore/Verifier.cpp

@@ -418,22 +418,10 @@ void Verifier::VerifyParamAttrs(const FunctionType *FT,
               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);

test/Transforms/InstCombine/2008-01-06-BitCastAttributes.ll

@@ -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
+}