When transforming a call to a bitcast function into

a direct call with cast parameters and cast return
value (if any), instcombine was prepared to cast any
non-void return value into any other, whether castable
or not.  Add a new predicate for testing whether casting
is valid, and check it both for the return value and
(as a cleanup) for the parameters.


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

5 files changed, 105 insertions, 47 deletions

diff --git a/include/llvm/InstrTypes.h b/include/llvm/InstrTypes.h
index dbaca74470..7441a104b5 100644
--- a/include/llvm/InstrTypes.h
+++ b/include/llvm/InstrTypes.h
@@ -398,8 +398,14 @@ public:
     BasicBlock *InsertAtEnd  ///< The block to insert the instruction into
   );
 
+  /// @brief Check whether it is valid to call getCastOpcode for these types.
+  static bool isCastable(
+    const Type *SrcTy, ///< The Type from which the value should be cast.
+    const Type *DestTy ///< The Type to which the value should be cast.
+  );
+
   /// Returns the opcode necessary to cast Val into Ty using usual casting
-  /// rules. 
+  /// rules.
   /// @brief Infer the opcode for cast operand and type
   static Instruction::CastOps getCastOpcode(
     const Value *Val, ///< The value to cast
diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp
index 1e12c79e68..4fcd47a870 100644
--- a/lib/Transforms/Scalar/InstructionCombining.cpp
+++ b/lib/Transforms/Scalar/InstructionCombining.cpp
@@ -8082,11 +8082,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
   const FunctionType *FT = Callee->getFunctionType();
   const Type *OldRetTy = Caller->getType();
 
-  const ParamAttrsList* CallerPAL = 0;
-  if (CallInst *CallerCI = dyn_cast<CallInst>(Caller))
-    CallerPAL = CallerCI->getParamAttrs();
-  else if (InvokeInst *CallerII = dyn_cast<InvokeInst>(Caller))
-    CallerPAL = CallerII->getParamAttrs();
+  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.
@@ -8101,6 +8097,12 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
           TD->getIntPtrType() == OldRetTy))
       return false;   // Cannot transform this return value.
 
+    if (!Caller->use_empty() &&
+        !CastInst::isCastable(FT->getReturnType(), OldRetTy) &&
+        // void -> non-void is handled specially
+        FT->getReturnType() != Type::VoidTy)
+      return false;   // Cannot transform this return 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
@@ -8122,9 +8124,13 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
   for (unsigned i = 0, e = NumCommonArgs; i != e; ++i, ++AI) {
     const Type *ParamTy = FT->getParamType(i);
     const Type *ActTy = (*AI)->getType();
+
+    if (!CastInst::isCastable(ActTy, ParamTy))
+      return false;
+
     ConstantInt *c = dyn_cast<ConstantInt>(*AI);
-    //Some conversions are safe even if we do not have a body.
-    //Either we can cast directly, or we can upconvert the argument
+    // Some conversions are safe even if we do not have a body.
+    // Either we can cast directly, or we can upconvert the argument
     bool isConvertible = ActTy == ParamTy ||
       (isa<PointerType>(ParamTy) && isa<PointerType>(ActTy)) ||
       (ParamTy->isInteger() && ActTy->isInteger() &&
@@ -8132,40 +8138,6 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
       (c && ParamTy->getPrimitiveSizeInBits() >= ActTy->getPrimitiveSizeInBits()
        && c->getValue().isStrictlyPositive());
     if (Callee->isDeclaration() && !isConvertible) return false;
-
-    // Most other conversions can be done if we have a body, even if these
-    // lose information, e.g. int->short.
-    // Some conversions cannot be done at all, e.g. float to pointer.
-    // Logic here parallels CastInst::getCastOpcode (the design there
-    // requires legality checks like this be done before calling it).
-    if (ParamTy->isInteger()) {
-      if (const VectorType *VActTy = dyn_cast<VectorType>(ActTy)) {
-        if (VActTy->getBitWidth() != ParamTy->getPrimitiveSizeInBits())
-          return false;
-      }
-      if (!ActTy->isInteger() && !ActTy->isFloatingPoint() &&
-          !isa<PointerType>(ActTy))
-        return false;
-    } else if (ParamTy->isFloatingPoint()) {
-      if (const VectorType *VActTy = dyn_cast<VectorType>(ActTy)) {
-        if (VActTy->getBitWidth() != ParamTy->getPrimitiveSizeInBits())
-          return false;
-      }
-      if (!ActTy->isInteger() && !ActTy->isFloatingPoint())
-        return false;
-    } else if (const VectorType *VParamTy = dyn_cast<VectorType>(ParamTy)) {
-      if (const VectorType *VActTy = dyn_cast<VectorType>(ActTy)) {
-        if (VActTy->getBitWidth() != VParamTy->getBitWidth())
-          return false;
-      }
-      if (VParamTy->getBitWidth() != ActTy->getPrimitiveSizeInBits())      
-        return false;
-    } else if (isa<PointerType>(ParamTy)) {
-      if (!ActTy->isInteger() && !isa<PointerType>(ActTy))
-        return false;
-    } else {
-      return false;
-    }
   }
 
   if (FT->getNumParams() < NumActualArgs && !FT->isVarArg() &&
@@ -8238,12 +8210,11 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
 
   // Insert a cast of the return type as necessary.
   Value *NV = NC;
-  if (Caller->getType() != NV->getType() && !Caller->use_empty()) {
+  if (OldRetTy != NV->getType() && !Caller->use_empty()) {
     if (NV->getType() != Type::VoidTy) {
-      const Type *CallerTy = Caller->getType();
       Instruction::CastOps opcode = CastInst::getCastOpcode(NC, false, 
-                                                            CallerTy, false);
-      NV = NC = CastInst::create(opcode, NC, CallerTy, "tmp");
+                                                            OldRetTy, false);
+      NV = NC = CastInst::create(opcode, NC, OldRetTy, "tmp");
 
       // If this is an invoke instruction, we should insert it after the first
       // non-phi, instruction in the normal successor block.
diff --git a/lib/VMCore/Instructions.cpp b/lib/VMCore/Instructions.cpp
index 5179f04f01..d8bcb31a47 100644
--- a/lib/VMCore/Instructions.cpp
+++ b/lib/VMCore/Instructions.cpp
@@ -1895,12 +1895,70 @@ CastInst *CastInst::createFPCast(Value *C, const Type *Ty,
   return create(opcode, C, Ty, Name, InsertAtEnd);
 }
 
+// Check whether it is valid to call getCastOpcode for these types.
+// This routine must be kept in sync with getCastOpcode.
+bool CastInst::isCastable(const Type *SrcTy, const Type *DestTy) {
+  if (!SrcTy->isFirstClassType() || !DestTy->isFirstClassType())
+    return false;
+
+  if (SrcTy == DestTy)
+    return true;
+
+  // Get the bit sizes, we'll need these
+  unsigned SrcBits = SrcTy->getPrimitiveSizeInBits();   // 0 for ptr/vector
+  unsigned DestBits = DestTy->getPrimitiveSizeInBits(); // 0 for ptr/vector
+
+  // Run through the possibilities ...
+  if (DestTy->isInteger()) {                      // Casting to integral
+    if (SrcTy->isInteger()) {                     // Casting from integral
+        return true;
+    } else if (SrcTy->isFloatingPoint()) {        // Casting from floating pt
+      return true;
+    } else if (const VectorType *PTy = dyn_cast<VectorType>(SrcTy)) {
+                                                  // Casting from vector
+      return DestBits == PTy->getBitWidth();
+    } else {                                      // Casting from something else
+      return isa<PointerType>(SrcTy);
+    }
+  } else if (DestTy->isFloatingPoint()) {         // Casting to floating pt
+    if (SrcTy->isInteger()) {                     // Casting from integral
+      return true;
+    } else if (SrcTy->isFloatingPoint()) {        // Casting from floating pt
+      return true;
+    } else if (const VectorType *PTy = dyn_cast<VectorType>(SrcTy)) {
+                                                  // Casting from vector
+      return DestBits == PTy->getBitWidth();
+    } else {                                      // Casting from something else
+      return false;
+    }
+  } else if (const VectorType *DestPTy = dyn_cast<VectorType>(DestTy)) {
+                                                   // Casting to vector
+    if (const VectorType *SrcPTy = dyn_cast<VectorType>(SrcTy)) {
+                                                   // Casting from vector
+      return DestPTy->getBitWidth() == SrcPTy->getBitWidth();
+    } else {                                       // Casting from something else
+      return DestPTy->getBitWidth() == SrcBits;
+    }
+  } else if (isa<PointerType>(DestTy)) {           // Casting to pointer
+    if (isa<PointerType>(SrcTy)) {                 // Casting from pointer
+      return true;
+    } else if (SrcTy->isInteger()) {               // Casting from integral
+      return true;
+    } else {                                       // Casting from something else
+      return false;
+    }
+  } else {                                         // Casting to something else
+    return false;
+  }
+}
+
 // Provide a way to get a "cast" where the cast opcode is inferred from the 
 // types and size of the operand. This, basically, is a parallel of the 
 // logic in the castIsValid function below.  This axiom should hold:
 //   castIsValid( getCastOpcode(Val, Ty), Val, Ty)
 // should not assert in castIsValid. In other words, this produces a "correct"
 // casting opcode for the arguments passed to it.
+// This routine must be kept in sync with isCastable.
 Instruction::CastOps
 CastInst::getCastOpcode(
   const Value *Src, bool SrcIsSigned, const Type *DestTy, bool DestIsSigned) {
@@ -1909,6 +1967,9 @@ CastInst::getCastOpcode(
   unsigned SrcBits = SrcTy->getPrimitiveSizeInBits();   // 0 for ptr/vector
   unsigned DestBits = DestTy->getPrimitiveSizeInBits(); // 0 for ptr/vector
 
+  assert(SrcTy->isFirstClassType() && DestTy->isFirstClassType() &&
+         "Only first class types are castable!");
+
   // Run through the possibilities ...
   if (DestTy->isInteger()) {                       // Casting to integral
     if (SrcTy->isInteger()) {                      // Casting from integral
@@ -2050,7 +2111,7 @@ CastInst::castIsValid(Instruction::CastOps op, Value *S, const Type *DstTy) {
     if (isa<PointerType>(SrcTy) != isa<PointerType>(DstTy))
       return false;
 
-    // Now we know we're not dealing with a pointer/non-poiner mismatch. In all
+    // Now we know we're not dealing with a pointer/non-pointer mismatch. In all
     // these cases, the cast is okay if the source and destination bit widths
     // are identical.
     return SrcBitSize == DstBitSize;
diff --git a/test/Transforms/InstCombine/2008-01-06-CastCrash.ll b/test/Transforms/InstCombine/2008-01-06-CastCrash.ll
new file mode 100644
index 0000000000..1d816d4b99
--- /dev/null
+++ b/test/Transforms/InstCombine/2008-01-06-CastCrash.ll
@@ -0,0 +1,10 @@
+; RUN: llvm-as < %s | opt -instcombine -disable-output
+
+define <2 x i32> @f() {
+	ret <2 x i32> undef
+}
+
+define i32 @g() {
+	%x = call i32 bitcast (<2 x i32> ()* @f to i32 ()*)( )		; <i32> [#uses=1]
+	ret i32 %x
+}
diff --git a/test/Transforms/InstCombine/2008-01-06-VoidCast.ll b/test/Transforms/InstCombine/2008-01-06-VoidCast.ll
new file mode 100644
index 0000000000..015210a998
--- /dev/null
+++ b/test/Transforms/InstCombine/2008-01-06-VoidCast.ll
@@ -0,0 +1,10 @@
+; RUN: llvm-as < %s | opt -instcombine | llvm-dis | not grep bitcast
+
+define void @f(i16 %y) {
+	ret void
+}
+
+define i32 @g(i32 %y) {
+	%x = call i32 bitcast (void (i16)* @f to i32 (i32)*)( i32 %y )		; <i32> [#uses=1]
+	ret i32 %x
+}