diff --git a/include/llvm/Instruction.h b/include/llvm/Instruction.h index e06ae2d7112..1d1f0f42d12 100644 --- a/include/llvm/Instruction.h +++ b/include/llvm/Instruction.h @@ -168,13 +168,6 @@ public: bool isCommutative() const { return isCommutative(getOpcode()); } static bool isCommutative(unsigned op); - /// isTrapping - Return true if the instruction may trap. - /// - bool isTrapping() const { - return isTrapping(getOpcode()); - } - static bool isTrapping(unsigned op); - /// mayWriteToMemory - Return true if this instruction may modify memory. /// bool mayWriteToMemory() const; @@ -193,6 +186,17 @@ public: return mayWriteToMemory() || mayThrow(); } + /// isSafeToSpeculativelyExecute - Return true if the instruction does not + /// have any effects besides calculating the result and does not have + /// undefined behavior. Unlike in mayHaveSideEffects(), allocating memory + /// is considered an effect. + /// + /// This method only looks at the instruction itself and its operands, so if + /// this method returns true, it is safe to move the instruction as long as + /// the operands still dominate it. However, care must be taken with + /// instructions which read memory. + bool isSafeToSpeculativelyExecute() const; + /// Methods for support type inquiry through isa, cast, and dyn_cast: static inline bool classof(const Instruction *) { return true; } static inline bool classof(const Value *V) { diff --git a/lib/Analysis/LoopInfo.cpp b/lib/Analysis/LoopInfo.cpp index d350fa6f9eb..bef6bef3370 100644 --- a/lib/Analysis/LoopInfo.cpp +++ b/lib/Analysis/LoopInfo.cpp @@ -79,14 +79,9 @@ bool Loop::makeLoopInvariant(Instruction *I, bool &Changed, // Test if the value is already loop-invariant. if (isLoopInvariant(I)) return true; - // Don't hoist instructions with side-effects. - if (I->isTrapping()) + if (!I->isSafeToSpeculativelyExecute()) return false; - // Don't hoist PHI nodes. - if (isa(I)) - return false; - // Don't hoist allocation instructions. - if (isa(I)) + if (I->mayReadFromMemory()) return false; // Determine the insertion point, unless one was given. if (!InsertPt) { diff --git a/lib/Transforms/Scalar/LICM.cpp b/lib/Transforms/Scalar/LICM.cpp index 52dd06affe5..bdf7dee6620 100644 --- a/lib/Transforms/Scalar/LICM.cpp +++ b/lib/Transforms/Scalar/LICM.cpp @@ -623,7 +623,8 @@ void LICM::hoist(Instruction &I) { /// bool LICM::isSafeToExecuteUnconditionally(Instruction &Inst) { // If it is not a trapping instruction, it is always safe to hoist. - if (!Inst.isTrapping()) return true; + if (Inst.isSafeToSpeculativelyExecute()) + return true; // Otherwise we have to check to make sure that the instruction dominates all // of the exit blocks. If it doesn't, then there is a path out of the loop @@ -635,12 +636,6 @@ bool LICM::isSafeToExecuteUnconditionally(Instruction &Inst) { if (Inst.getParent() == CurLoop->getHeader()) return true; - // It's always safe to load from a global or alloca. - if (isa(Inst)) - if (isa(Inst.getOperand(0)) || - isa(Inst.getOperand(0))) - return true; - // Get the exit blocks for the current loop. SmallVector ExitBlocks; CurLoop->getExitBlocks(ExitBlocks); diff --git a/lib/Transforms/Scalar/TailDuplication.cpp b/lib/Transforms/Scalar/TailDuplication.cpp index 684b0963456..6d05fdf18c2 100644 --- a/lib/Transforms/Scalar/TailDuplication.cpp +++ b/lib/Transforms/Scalar/TailDuplication.cpp @@ -258,7 +258,8 @@ void TailDup::eliminateUnconditionalBranch(BranchInst *Branch) { while (!isa(BBI)) { Instruction *I = BBI++; - bool CanHoist = !I->isTrapping() && !I->mayHaveSideEffects(); + bool CanHoist = I->isSafeToSpeculativelyExecute() && + !I->mayReadFromMemory(); if (CanHoist) { for (unsigned op = 0, e = I->getNumOperands(); op != e; ++op) if (Instruction *OpI = dyn_cast(I->getOperand(op))) diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index 55e1bf225c2..3b6b77f5fb8 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -384,26 +384,15 @@ static bool DominatesMergePoint(Value *V, BasicBlock *BB, // Okay, it looks like the instruction IS in the "condition". Check to // see if its a cheap instruction to unconditionally compute, and if it // only uses stuff defined outside of the condition. If so, hoist it out. + if (!I->isSafeToSpeculativelyExecute()) + return false; + switch (I->getOpcode()) { default: return false; // Cannot hoist this out safely. case Instruction::Load: { - // We can hoist loads that are non-volatile and obviously cannot trap. - if (cast(I)->isVolatile()) - return false; - // FIXME: A computation of a constant can trap! - if (!isa(I->getOperand(0)) && - !isa(I->getOperand(0))) - return false; - // External weak globals may have address 0, so we can't load them. - Value *V2 = I->getOperand(0)->getUnderlyingObject(); - if (V2) { - GlobalVariable* GV = dyn_cast(V2); - if (GV && GV->hasExternalWeakLinkage()) - return false; - } - // Finally, we have to check to make sure there are no instructions - // before the load in its basic block, as we are going to hoist the loop - // out to its predecessor. + // We have to check to make sure there are no instructions before the + // load in its basic block, as we are going to hoist the loop out to + // its predecessor. BasicBlock::iterator IP = PBB->begin(); while (isa(IP)) IP++; diff --git a/lib/VMCore/Instruction.cpp b/lib/VMCore/Instruction.cpp index 99fd8d09e5b..e19ad1c16f9 100644 --- a/lib/VMCore/Instruction.cpp +++ b/lib/VMCore/Instruction.cpp @@ -14,6 +14,8 @@ #include "llvm/Type.h" #include "llvm/Instructions.h" #include "llvm/Function.h" +#include "llvm/Constants.h" +#include "llvm/GlobalVariable.h" #include "llvm/Support/CallSite.h" #include "llvm/Support/LeakDetector.h" using namespace llvm; @@ -365,24 +367,56 @@ bool Instruction::isCommutative(unsigned op) { } } -/// isTrapping - Return true if the instruction may trap. -/// -bool Instruction::isTrapping(unsigned op) { - switch(op) { - case UDiv: - case SDiv: - case FDiv: - case URem: - case SRem: - case FRem: - case Load: - case Store: - case Call: - case Invoke: - case VAArg: - case Free: - return true; +bool Instruction::isSafeToSpeculativelyExecute() const { + for (unsigned i = 0, e = getNumOperands(); i != e; ++i) + if (Constant *C = dyn_cast(getOperand(i))) + if (C->canTrap()) + return false; + + switch (getOpcode()) { default: + return true; + case UDiv: + case URem: { + // x / y is undefined if y == 0, but calcuations like x / 3 are safe. + ConstantInt *Op = dyn_cast(getOperand(1)); + return Op && !Op->isNullValue(); + } + case SDiv: + case SRem: { + // x / y is undefined if y == 0, and might be undefined if y == -1, + // but calcuations like x / 3 are safe. + ConstantInt *Op = dyn_cast(getOperand(1)); + return Op && !Op->isNullValue() && !Op->isAllOnesValue(); + } + case Load: { + if (cast(this)->isVolatile()) + return false; + if (isa(getOperand(0))) + return true; + if (GlobalVariable *GV = dyn_cast(getOperand(0))) + return !GV->hasExternalWeakLinkage(); + // FIXME: Handle cases involving GEPs. We have to be careful because + // a load of a out-of-bounds GEP has undefined behavior. return false; } + case Call: + return false; // The called function could have undefined behavior or + // side-effects. + // FIXME: We should special-case some intrinsics (bswap, + // overflow-checking arithmetic, etc.) + case VAArg: + case Alloca: + case Malloc: + case Invoke: + case PHI: + case Store: + case Free: + case Ret: + case Br: + case Switch: + case Unwind: + case Unreachable: + return false; // Misc instructions which have effects + } }