From a9d0c9dc58855a5f01dcc5c85c89fd3fc737d3e8 Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Sun, 6 Jan 2008 10:12:28 +0000 Subject: [PATCH] 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 --- include/llvm/InstrTypes.h | 8 ++- .../Scalar/InstructionCombining.cpp | 61 +++++------------- lib/VMCore/Instructions.cpp | 63 ++++++++++++++++++- .../InstCombine/2008-01-06-CastCrash.ll | 10 +++ .../InstCombine/2008-01-06-VoidCast.ll | 10 +++ 5 files changed, 105 insertions(+), 47 deletions(-) create mode 100644 test/Transforms/InstCombine/2008-01-06-CastCrash.ll create mode 100644 test/Transforms/InstCombine/2008-01-06-VoidCast.ll diff --git a/include/llvm/InstrTypes.h b/include/llvm/InstrTypes.h index dbaca744708..7441a104b56 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 1e12c79e688..4fcd47a870c 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(Caller)) - CallerPAL = CallerCI->getParamAttrs(); - else if (InvokeInst *CallerII = dyn_cast(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(*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(ParamTy) && isa(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(ActTy)) { - if (VActTy->getBitWidth() != ParamTy->getPrimitiveSizeInBits()) - return false; - } - if (!ActTy->isInteger() && !ActTy->isFloatingPoint() && - !isa(ActTy)) - return false; - } else if (ParamTy->isFloatingPoint()) { - if (const VectorType *VActTy = dyn_cast(ActTy)) { - if (VActTy->getBitWidth() != ParamTy->getPrimitiveSizeInBits()) - return false; - } - if (!ActTy->isInteger() && !ActTy->isFloatingPoint()) - return false; - } else if (const VectorType *VParamTy = dyn_cast(ParamTy)) { - if (const VectorType *VActTy = dyn_cast(ActTy)) { - if (VActTy->getBitWidth() != VParamTy->getBitWidth()) - return false; - } - if (VParamTy->getBitWidth() != ActTy->getPrimitiveSizeInBits()) - return false; - } else if (isa(ParamTy)) { - if (!ActTy->isInteger() && !isa(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 5179f04f011..d8bcb31a476 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(SrcTy)) { + // Casting from vector + return DestBits == PTy->getBitWidth(); + } else { // Casting from something else + return isa(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(SrcTy)) { + // Casting from vector + return DestBits == PTy->getBitWidth(); + } else { // Casting from something else + return false; + } + } else if (const VectorType *DestPTy = dyn_cast(DestTy)) { + // Casting to vector + if (const VectorType *SrcPTy = dyn_cast(SrcTy)) { + // Casting from vector + return DestPTy->getBitWidth() == SrcPTy->getBitWidth(); + } else { // Casting from something else + return DestPTy->getBitWidth() == SrcBits; + } + } else if (isa(DestTy)) { // Casting to pointer + if (isa(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(SrcTy) != isa(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 00000000000..1d816d4b996 --- /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 ()*)( ) ; [#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 00000000000..015210a9983 --- /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 ) ; [#uses=1] + ret i32 %x +}