When simplifying a call to a bitcast function, tighten up

the conditions for performing the transform when only the
function declaration is available: no longer allow turning
i32 into i64 for example.  Only allow changing between
pointer types, and between pointer types and integers of
the same size.  For return values ptr -> intptr was already
allowed; I added ptr -> ptr and intptr -> ptr while there.
As shown by a recent objc testcase, changing the way
parameters/return values are passed can be fatal when calling
code written in assembler that directly manipulates call
arguments and return values unless the transform has no
impact on the way they are passed at the codegen level.
While it is possible to imagine an ABI that treats integers
of pointer size differently to pointers, I don't think LLVM
supports any so the transform should now be safe while still
being useful.

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

diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp
index 89c49c1a6e4537d4c8b237d76dff14b771a82a29..bb15e3504cc2deebc90d85340e0a0e20d50b2476 100644 (file)
--- a/lib/Transforms/Scalar/InstructionCombining.cpp
+++ b/lib/Transforms/Scalar/InstructionCombining.cpp
@@ -9450,27 +9450,28 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
   //
   const FunctionType *FT = Callee->getFunctionType();
   const Type *OldRetTy = Caller->getType();
+  const Type *NewRetTy = FT->getReturnType();
 
-  if (isa<StructType>(FT->getReturnType()))
+  if (isa<StructType>(NewRetTy))
     return false; // TODO: Handle multiple return values.
 
   // Check to see if we are changing the return type...
-  if (OldRetTy != FT->getReturnType()) {
+  if (OldRetTy != NewRetTy) {
     if (Callee->isDeclaration() &&
-        // Conversion is ok if changing from pointer to int of same size.
-        !(isa<PointerType>(FT->getReturnType()) &&
-          TD->getIntPtrType() == OldRetTy))
+        // Conversion is ok if changing from one pointer type to another or from
+        // a pointer to an integer of the same size.
+        !((isa<PointerType>(OldRetTy) || OldRetTy == TD->getIntPtrType()) &&
+          isa<PointerType>(NewRetTy) || NewRetTy == TD->getIntPtrType()))
       return false;   // Cannot transform this return value.
 
     if (!Caller->use_empty() &&
         // void -> non-void is handled specially
-        FT->getReturnType() != Type::VoidTy &&
-        !CastInst::isCastable(FT->getReturnType(), OldRetTy))
+        NewRetTy != Type::VoidTy && !CastInst::isCastable(NewRetTy, OldRetTy))
       return false;   // Cannot transform this return value.
 
     if (!CallerPAL.isEmpty() && !Caller->use_empty()) {
       ParameterAttributes RAttrs = CallerPAL.getParamAttrs(0);
-      if (RAttrs & ParamAttr::typeIncompatible(FT->getReturnType()))
+      if (RAttrs & ParamAttr::typeIncompatible(NewRetTy))
         return false;   // Attribute not compatible with transformed value.
     }
 
@@ -9502,15 +9503,11 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     if (CallerPAL.getParamAttrs(i + 1) & ParamAttr::typeIncompatible(ParamTy))
       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.
-    // Either we can cast directly, or we can upconvert the argument
+    // Converting from one pointer type to another or between a pointer and an
+    // integer of the same size is safe even if we do not have a body.
     bool isConvertible = ActTy == ParamTy ||
-      (isa<PointerType>(ParamTy) && isa<PointerType>(ActTy)) ||
-      (ParamTy->isInteger() && ActTy->isInteger() &&
-       ParamTy->getPrimitiveSizeInBits() >= ActTy->getPrimitiveSizeInBits()) ||
-      (c && ParamTy->getPrimitiveSizeInBits() >= ActTy->getPrimitiveSizeInBits()
-       && c->getValue().isStrictlyPositive());
+      ((isa<PointerType>(ParamTy) || ParamTy == TD->getIntPtrType()) &&
+       (isa<PointerType>(ActTy) || ActTy == TD->getIntPtrType()));
     if (Callee->isDeclaration() && !isConvertible) return false;
   }
 
@@ -9543,7 +9540,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
 
   // 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::typeIncompatible(FT->getReturnType());
+  RAttrs &= ~ParamAttr::typeIncompatible(NewRetTy);
 
   // Add the new return attributes.
   if (RAttrs)
@@ -9598,7 +9595,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {
     }
   }
 
-  if (FT->getReturnType() == Type::VoidTy)
+  if (NewRetTy == Type::VoidTy)
     Caller->setName("");   // Void type should not have a name.
 
   const PAListPtr &NewCallerPAL = PAListPtr::get(attrVec.begin(),attrVec.end());

diff --git a/test/Transforms/InstCombine/apint-call-cast-target.ll b/test/Transforms/InstCombine/apint-call-cast-target.ll
index 0f87a53160f517f038e30baef54f7ed27955e08e..3688fbe1f2939ab31992034f1844af14fd19e0ad 100644 (file)
--- a/test/Transforms/InstCombine/apint-call-cast-target.ll
+++ b/test/Transforms/InstCombine/apint-call-cast-target.ll
@@ -10,4 +10,8 @@ entry:
        ret i32 %tmp
 }
 
-declare i7* @ctime(i999*)
+define i7* @ctime(i999*) {
+entry:
+       %tmp = call i7* bitcast (i32 ()* @main to i7* ()*)( )
+       ret i7* %tmp
+}