[llvm-commits] [llvm] r75567 - /llvm/trunk/lib/VMCore/Instruction.cpp
Eli Friedman
eli.friedman at gmail.com
Wed Jul 15 22:21:44 PDT 2009
On Wed, Jul 15, 2009 at 9:38 PM, Nick Lewycky<nicholas at mxc.ca> wrote:
>> Perhaps mayHaveSideEffects() should be renamed to
>> mayHaveDefinedSideEffects(), and isTrapping() should be renamed to
>> mayHaveUndefinedBehavior()?
>
> mayHaveUndefinedBehavior is a great choice here. That perfectly captures
> what it's doing.
Attached is another suggestion. Replaces isTrapping with a similar
method named isSafeToSpeculativelyExecute. Hopefully, this should be
perfectly clear.
-Eli
-------------- next part --------------
Index: include/llvm/Instruction.h
===================================================================
--- include/llvm/Instruction.h (revision 75894)
+++ include/llvm/Instruction.h (working copy)
@@ -168,13 +168,6 @@
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 @@
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) {
Index: lib/VMCore/Instruction.cpp
===================================================================
--- lib/VMCore/Instruction.cpp (revision 75894)
+++ lib/VMCore/Instruction.cpp (working copy)
@@ -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,53 @@
}
}
-/// 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:
+bool Instruction::isSafeToSpeculativelyExecute() const {
+ for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
+ if (Constant *C = dyn_cast<Constant>(getOperand(i)))
+ if (C->canTrap())
+ return false;
+
+ // An instruction with a void return type either has side effects or is dead.
+ if (getType()->getTypeID() == Type::VoidTyID)
+ return false;
+
+ switch (getOpcode()) {
+ default:
return true;
- default:
+ case Instruction::UDiv:
+ case Instruction::URem: {
+ // x / y is undefined if y == 0, but calcuations like x / 3 are safe.
+ ConstantInt *Op = dyn_cast<ConstantInt>(getOperand(1));
+ return Op && !Op->isNullValue();
+ }
+ case Instruction::SDiv:
+ case Instruction::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<ConstantInt>(getOperand(1));
+ return Op && !Op->isNullValue() && !Op->isAllOnesValue();
+ }
+ case Instruction::Load: {
+ if (cast<LoadInst>(this)->isVolatile())
+ return false;
+ if (isa<AllocationInst>(getOperand(0)))
+ return true;
+ if (GlobalVariable *GV = dyn_cast<GlobalVariable>(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 Instruction::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 Instruction::VAArg:
+ case Instruction::Alloca:
+ case Instruction::Malloc:
+ case Instruction::Invoke:
+ case Instruction::PHI:
+ return false; // Misc instructions which have effects
+ }
}
Index: lib/Analysis/LoopInfo.cpp
===================================================================
--- lib/Analysis/LoopInfo.cpp (revision 75894)
+++ lib/Analysis/LoopInfo.cpp (working copy)
@@ -79,15 +79,10 @@
// 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<PHINode>(I))
+ if (I->mayReadFromMemory())
return false;
- // Don't hoist allocation instructions.
- if (isa<AllocationInst>(I))
- return false;
// Determine the insertion point, unless one was given.
if (!InsertPt) {
BasicBlock *Preheader = getLoopPreheader();
Index: lib/Transforms/Scalar/LICM.cpp
===================================================================
--- lib/Transforms/Scalar/LICM.cpp (revision 75894)
+++ lib/Transforms/Scalar/LICM.cpp (working copy)
@@ -623,7 +623,8 @@
///
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 @@
if (Inst.getParent() == CurLoop->getHeader())
return true;
- // It's always safe to load from a global or alloca.
- if (isa<LoadInst>(Inst))
- if (isa<AllocationInst>(Inst.getOperand(0)) ||
- isa<GlobalVariable>(Inst.getOperand(0)))
- return true;
-
// Get the exit blocks for the current loop.
SmallVector<BasicBlock*, 8> ExitBlocks;
CurLoop->getExitBlocks(ExitBlocks);
Index: lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- lib/Transforms/Utils/SimplifyCFG.cpp (revision 75894)
+++ lib/Transforms/Utils/SimplifyCFG.cpp (working copy)
@@ -384,26 +384,15 @@
// 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<LoadInst>(I)->isVolatile())
- return false;
- // FIXME: A computation of a constant can trap!
- if (!isa<AllocaInst>(I->getOperand(0)) &&
- !isa<Constant>(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<GlobalVariable>(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<DbgInfoIntrinsic>(IP))
IP++;
More information about the llvm-commits
mailing list