From f286f6fd93d569befe6e77c94a947e6e04e95685 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Mon, 10 Dec 2007 22:53:04 +0000 Subject: [PATCH] Fix PR1850 by removing an unsafe transformation from VMCore/ConstantFold.cpp. Reimplement the xform in Analysis/ConstantFolding.cpp where we can use targetdata to validate that it is safe. While I'm in there, fix some const correctness issues and generalize the interface to the "operand folder". git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@44817 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/ConstantFolding.h | 20 ++-- lib/Analysis/ConstantFolding.cpp | 100 ++++++++++++++---- lib/Analysis/ScalarEvolution.cpp | 16 ++- lib/Transforms/Utils/CloneFunction.cpp | 8 +- lib/VMCore/ConstantFold.cpp | 21 ++-- test/Assembler/ConstantExprFold.llx | 3 - .../2007-12-10-ConstFoldCompare.ll | 9 ++ 7 files changed, 134 insertions(+), 43 deletions(-) create mode 100644 test/Transforms/InstCombine/2007-12-10-ConstFoldCompare.ll diff --git a/include/llvm/Analysis/ConstantFolding.h b/include/llvm/Analysis/ConstantFolding.h index 9c19f111d16..654eee299f5 100644 --- a/include/llvm/Analysis/ConstantFolding.h +++ b/include/llvm/Analysis/ConstantFolding.h @@ -21,6 +21,7 @@ namespace llvm { class Instruction; class TargetData; class Function; + class Type; /// ConstantFoldInstruction - Attempt to constant fold the specified /// instruction. If successful, the constant result is returned, if not, null @@ -35,12 +36,17 @@ Constant *ConstantFoldInstruction(Instruction *I, const TargetData *TD = 0); /// fold instructions like loads and stores, which have no constant expression /// form. /// -Constant *ConstantFoldInstOperands( - const Instruction *I, ///< The model instruction - Constant** Ops, ///< The array of constant operands to use. - unsigned NumOps, ///< The number of operands provided. - const TargetData *TD = 0 ///< Optional target information. -); +Constant *ConstantFoldInstOperands(unsigned Opcode, const Type *DestTy, + Constant*const * Ops, unsigned NumOps, + const TargetData *TD = 0); + +/// ConstantFoldCompareInstOperands - Attempt to constant fold a compare +/// instruction (icmp/fcmp) with the specified operands. If it fails, it +/// returns a constant expression of the specified operands. +/// +Constant *ConstantFoldCompareInstOperands(unsigned Predicate, + Constant*const * Ops, unsigned NumOps, + const TargetData *TD = 0); /// ConstantFoldLoadThroughGEPConstantExpr - Given a constant and a @@ -55,7 +61,7 @@ bool canConstantFoldCallTo(Function *F); /// ConstantFoldCall - Attempt to constant fold a call to the specified function /// with the specified arguments, returning null if unsuccessful. Constant * -ConstantFoldCall(Function *F, Constant** Operands, unsigned NumOperands); +ConstantFoldCall(Function *F, Constant* const* Operands, unsigned NumOperands); } #endif diff --git a/lib/Analysis/ConstantFolding.cpp b/lib/Analysis/ConstantFolding.cpp index 494c94ed7b4..8c398462d40 100644 --- a/lib/Analysis/ConstantFolding.cpp +++ b/lib/Analysis/ConstantFolding.cpp @@ -55,7 +55,8 @@ static bool IsConstantOffsetFromGlobal(Constant *C, GlobalValue *&GV, if (CE->getOpcode() == Instruction::GetElementPtr) { // Cannot compute this if the element type of the pointer is missing size // info. - if (!cast(CE->getOperand(0)->getType())->getElementType()->isSized()) + if (!cast(CE->getOperand(0)->getType()) + ->getElementType()->isSized()) return false; // If the base isn't a global+constant, we aren't either. @@ -117,7 +118,7 @@ static Constant *SymbolicallyEvaluateBinop(unsigned Opc, Constant *Op0, /// SymbolicallyEvaluateGEP - If we can symbolically evaluate the specified GEP /// constant expression, do so. -static Constant *SymbolicallyEvaluateGEP(Constant** Ops, unsigned NumOps, +static Constant *SymbolicallyEvaluateGEP(Constant* const* Ops, unsigned NumOps, const Type *ResultTy, const TargetData *TD) { Constant *Ptr = Ops[0]; @@ -181,7 +182,12 @@ Constant *llvm::ConstantFoldInstruction(Instruction *I, const TargetData *TD) { else return 0; // All operands not constant! - return ConstantFoldInstOperands(I, &Ops[0], Ops.size(), TD); + if (const CmpInst *CI = dyn_cast(I)) + return ConstantFoldCompareInstOperands(CI->getPredicate(), + &Ops[0], Ops.size(), TD); + else + return ConstantFoldInstOperands(I->getOpcode(), I->getType(), + &Ops[0], Ops.size(), TD); } /// ConstantFoldInstOperands - Attempt to constant fold an instruction with the @@ -190,23 +196,19 @@ Constant *llvm::ConstantFoldInstruction(Instruction *I, const TargetData *TD) { /// attempting to fold instructions like loads and stores, which have no /// constant expression form. /// -Constant *llvm::ConstantFoldInstOperands(const Instruction* I, - Constant** Ops, unsigned NumOps, +Constant *llvm::ConstantFoldInstOperands(unsigned Opcode, const Type *DestTy, + Constant* const* Ops, unsigned NumOps, const TargetData *TD) { - unsigned Opc = I->getOpcode(); - const Type *DestTy = I->getType(); - // Handle easy binops first. - if (isa(I)) { + if (Instruction::isBinaryOp(Opcode)) { if (isa(Ops[0]) || isa(Ops[1])) - if (Constant *C = SymbolicallyEvaluateBinop(I->getOpcode(), Ops[0], - Ops[1], TD)) + if (Constant *C = SymbolicallyEvaluateBinop(Opcode, Ops[0], Ops[1], TD)) return C; - return ConstantExpr::get(Opc, Ops[0], Ops[1]); + return ConstantExpr::get(Opcode, Ops[0], Ops[1]); } - switch (Opc) { + switch (Opcode) { default: return 0; case Instruction::Call: if (Function *F = dyn_cast(Ops[0])) @@ -215,8 +217,7 @@ Constant *llvm::ConstantFoldInstOperands(const Instruction* I, return 0; case Instruction::ICmp: case Instruction::FCmp: - return ConstantExpr::getCompare(cast(I)->getPredicate(), Ops[0], - Ops[1]); + assert(0 &&"This function is invalid for compares: no predicate specified"); case Instruction::PtrToInt: // If the input is a inttoptr, eliminate the pair. This requires knowing // the width of a pointer, so it can't be done in ConstantExpr::getCast. @@ -229,7 +230,7 @@ Constant *llvm::ConstantFoldInstOperands(const Instruction* I, TD->getPointerSizeInBits())); Input = ConstantExpr::getAnd(Input, Mask); // Do a zext or trunc to get to the dest size. - return ConstantExpr::getIntegerCast(Input, I->getType(), false); + return ConstantExpr::getIntegerCast(Input, DestTy, false); } } // FALL THROUGH. @@ -244,7 +245,7 @@ Constant *llvm::ConstantFoldInstOperands(const Instruction* I, case Instruction::FPToUI: case Instruction::FPToSI: case Instruction::BitCast: - return ConstantExpr::getCast(Opc, Ops[0], DestTy); + return ConstantExpr::getCast(Opcode, Ops[0], DestTy); case Instruction::Select: return ConstantExpr::getSelect(Ops[0], Ops[1], Ops[2]); case Instruction::ExtractElement: @@ -254,13 +255,73 @@ Constant *llvm::ConstantFoldInstOperands(const Instruction* I, case Instruction::ShuffleVector: return ConstantExpr::getShuffleVector(Ops[0], Ops[1], Ops[2]); case Instruction::GetElementPtr: - if (Constant *C = SymbolicallyEvaluateGEP(Ops, NumOps, I->getType(), TD)) + if (Constant *C = SymbolicallyEvaluateGEP(Ops, NumOps, DestTy, TD)) return C; return ConstantExpr::getGetElementPtr(Ops[0], Ops+1, NumOps-1); } } +/// ConstantFoldCompareInstOperands - Attempt to constant fold a compare +/// instruction (icmp/fcmp) with the specified operands. If it fails, it +/// returns a constant expression of the specified operands. +/// +Constant *llvm::ConstantFoldCompareInstOperands(unsigned Predicate, + Constant*const * Ops, + unsigned NumOps, + const TargetData *TD) { + // fold: icmp (inttoptr x), null -> icmp x, 0 + // fold: icmp (ptrtoint x), 0 -> icmp x, null + // fold: icmp (inttoptr x), (inttoptr y) -> icmp x, y + // fold: icmp (ptrtoint x), (ptrtoint y) -> icmp x, y + // + // ConstantExpr::getCompare cannot do this, because it doesn't have TD + // around to know if bit truncation is happening. + if (ConstantExpr *CE0 = dyn_cast(Ops[0])) { + if (TD && Ops[1]->isNullValue()) { + const Type *IntPtrTy = TD->getIntPtrType(); + if (CE0->getOpcode() == Instruction::IntToPtr) { + // Convert the integer value to the right size to ensure we get the + // proper extension or truncation. + Constant *C = ConstantExpr::getIntegerCast(CE0->getOperand(0), + IntPtrTy, false); + Constant *NewOps[] = { C, Constant::getNullValue(C->getType()) }; + return ConstantFoldCompareInstOperands(Predicate, NewOps, 2, TD); + } + + // Only do this transformation if the int is intptrty in size, otherwise + // there is a truncation or extension that we aren't modeling. + if (CE0->getOpcode() == Instruction::PtrToInt && + CE0->getType() == IntPtrTy) { + Constant *C = CE0->getOperand(0); + Constant *NewOps[] = { C, Constant::getNullValue(C->getType()) }; + // FIXME! + return ConstantFoldCompareInstOperands(Predicate, NewOps, 2, TD); + } + } + + if (TD && isa(Ops[1]) && + cast(Ops[1])->getOpcode() == CE0->getOpcode()) { + const Type *IntPtrTy = TD->getIntPtrType(); + // Only do this transformation if the int is intptrty in size, otherwise + // there is a truncation or extension that we aren't modeling. + if ((CE0->getOpcode() == Instruction::IntToPtr && + CE0->getOperand(0)->getType() == IntPtrTy && + CE0->getOperand(1)->getType() == IntPtrTy) || + (CE0->getOpcode() == Instruction::PtrToInt && + CE0->getType() == IntPtrTy && + CE0->getOperand(0)->getType() == CE0->getOperand(1)->getType())) { + Constant *NewOps[] = { + CE0->getOperand(0), cast(Ops[1])->getOperand(0) + }; + return ConstantFoldCompareInstOperands(Predicate, NewOps, 2, TD); + } + } + } + return ConstantExpr::getCompare(Predicate, Ops[0], Ops[1]); +} + + /// ConstantFoldLoadThroughGEPConstantExpr - Given a constant and a /// getelementptr constantexpr, return the constant value being addressed by the /// constant expression, or null if something is funny and we can't decide. @@ -438,7 +499,8 @@ static Constant *ConstantFoldBinaryFP(double (*NativeFP)(double, double), /// with the specified arguments, returning null if unsuccessful. Constant * -llvm::ConstantFoldCall(Function *F, Constant** Operands, unsigned NumOperands) { +llvm::ConstantFoldCall(Function *F, + Constant* const* Operands, unsigned NumOperands) { const ValueName *NameVal = F->getValueName(); if (NameVal == 0) return 0; const char *Str = NameVal->getKeyData(); diff --git a/lib/Analysis/ScalarEvolution.cpp b/lib/Analysis/ScalarEvolution.cpp index 558da230d19..a60a304330e 100644 --- a/lib/Analysis/ScalarEvolution.cpp +++ b/lib/Analysis/ScalarEvolution.cpp @@ -2043,7 +2043,12 @@ static Constant *EvaluateExpression(Value *V, Constant *PHIVal) { if (Operands[i] == 0) return 0; } - return ConstantFoldInstOperands(I, &Operands[0], Operands.size()); + if (const CmpInst *CI = dyn_cast(I)) + return ConstantFoldCompareInstOperands(CI->getPredicate(), + &Operands[0], Operands.size()); + else + return ConstantFoldInstOperands(I->getOpcode(), I->getType(), + &Operands[0], Operands.size()); } /// getConstantEvolutionLoopExitValue - If we know that the specified Phi is @@ -2213,7 +2218,14 @@ SCEVHandle ScalarEvolutionsImpl::getSCEVAtScope(SCEV *V, const Loop *L) { } } } - Constant *C =ConstantFoldInstOperands(I, &Operands[0], Operands.size()); + + Constant *C; + if (const CmpInst *CI = dyn_cast(I)) + C = ConstantFoldCompareInstOperands(CI->getPredicate(), + &Operands[0], Operands.size()); + else + C = ConstantFoldInstOperands(I->getOpcode(), I->getType(), + &Operands[0], Operands.size()); return SE.getUnknown(C); } } diff --git a/lib/Transforms/Utils/CloneFunction.cpp b/lib/Transforms/Utils/CloneFunction.cpp index f05085fca16..e85fe8e40a0 100644 --- a/lib/Transforms/Utils/CloneFunction.cpp +++ b/lib/Transforms/Utils/CloneFunction.cpp @@ -299,7 +299,13 @@ ConstantFoldMappedInstruction(const Instruction *I) { else return 0; // All operands not constant! - return ConstantFoldInstOperands(I, &Ops[0], Ops.size(), TD); + + if (const CmpInst *CI = dyn_cast(I)) + return ConstantFoldCompareInstOperands(CI->getPredicate(), + &Ops[0], Ops.size(), TD); + else + return ConstantFoldInstOperands(I->getOpcode(), I->getType(), + &Ops[0], Ops.size(), TD); } /// CloneAndPruneFunctionInto - This works exactly like CloneFunctionInto, diff --git a/lib/VMCore/ConstantFold.cpp b/lib/VMCore/ConstantFold.cpp index 95140f3fe1d..5e5bbeaae1f 100644 --- a/lib/VMCore/ConstantFold.cpp +++ b/lib/VMCore/ConstantFold.cpp @@ -985,20 +985,19 @@ static ICmpInst::Predicate evaluateICmpRelation(const Constant *V1, case Instruction::UIToFP: case Instruction::SIToFP: - case Instruction::IntToPtr: case Instruction::BitCast: case Instruction::ZExt: case Instruction::SExt: - case Instruction::PtrToInt: // If the cast is not actually changing bits, and the second operand is a // null pointer, do the comparison with the pre-casted value. if (V2->isNullValue() && (isa(CE1->getType()) || CE1->getType()->isInteger())) { - bool sgnd = CE1->getOpcode() == Instruction::ZExt ? false : - (CE1->getOpcode() == Instruction::SExt ? true : - (CE1->getOpcode() == Instruction::PtrToInt ? false : isSigned)); - return evaluateICmpRelation( - CE1Op0, Constant::getNullValue(CE1Op0->getType()), sgnd); + bool sgnd = isSigned; + if (CE1->getOpcode() == Instruction::ZExt) isSigned = false; + if (CE1->getOpcode() == Instruction::SExt) isSigned = true; + return evaluateICmpRelation(CE1Op0, + Constant::getNullValue(CE1Op0->getType()), + sgnd); } // If the dest type is a pointer type, and the RHS is a constantexpr cast @@ -1009,11 +1008,11 @@ static ICmpInst::Predicate evaluateICmpRelation(const Constant *V1, if (CE2->isCast() && isa(CE1->getType()) && CE1->getOperand(0)->getType() == CE2->getOperand(0)->getType() && CE1->getOperand(0)->getType()->isInteger()) { - bool sgnd = CE1->getOpcode() == Instruction::ZExt ? false : - (CE1->getOpcode() == Instruction::SExt ? true : - (CE1->getOpcode() == Instruction::PtrToInt ? false : isSigned)); + bool sgnd = isSigned; + if (CE1->getOpcode() == Instruction::ZExt) isSigned = false; + if (CE1->getOpcode() == Instruction::SExt) isSigned = true; return evaluateICmpRelation(CE1->getOperand(0), CE2->getOperand(0), - sgnd); + sgnd); } break; diff --git a/test/Assembler/ConstantExprFold.llx b/test/Assembler/ConstantExprFold.llx index b4d779d7946..02857439201 100644 --- a/test/Assembler/ConstantExprFold.llx +++ b/test/Assembler/ConstantExprFold.llx @@ -24,6 +24,3 @@ global bool setlt (int* getelementptr (%Ty* %B, long 0, uint 0), int* getelementptr (%Ty* %B, long 0, uint 1)) ; true ;global bool setne (long* %A, long* cast (%Ty* %B to long*)) ; true -global bool seteq ({ short }* cast (int 1 to { short }*), { short }* null) -global bool setlt ({ short }* cast (int 1 to { short }*), { short }* cast (int 2 to { short }*)) - diff --git a/test/Transforms/InstCombine/2007-12-10-ConstFoldCompare.ll b/test/Transforms/InstCombine/2007-12-10-ConstFoldCompare.ll new file mode 100644 index 00000000000..80df6fbdce3 --- /dev/null +++ b/test/Transforms/InstCombine/2007-12-10-ConstFoldCompare.ll @@ -0,0 +1,9 @@ +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32" +target triple = "i686-pc-linux-gnu" +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | not grep {ret i1 0} +; PR1850 + +define i1 @test() { + %cond = icmp ule i8* inttoptr (i64 4294967297 to i8*), inttoptr (i64 5 to i8*) + ret i1 %cond +} -- 2.34.1