[llvm-commits] [llvm] r138618 - in /llvm/trunk: lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/phi-undef-loadstore.ll

[Eli Friedman]

On Thu, Aug 25, 2011 at 6:22 PM, Benjamin Kramer
<benny.kra at googlemail.com> wrote:
> Author: d0k
> Date: Thu Aug 25 20:22:29 2011
> New Revision: 138618
>
> URL: http://llvm.org/viewvc/llvm-project?rev=138618&view=rev
> Log:
> SimplifyCFG: If we have a PHI node that can evaluate to NULL and do a load or store to the address returned by the PHI node then we can consider this incoming value as dead and remove the edge pointing there, unless there are instructions that can affect control flow executed in between.
>
> In theory this could be extended to other instructions, eg. division by zero, but it's likely that it will "miscompile" some code because people depend on div by zero not trapping. NULL pointer dereference usually leads to a crash so we should be on the safe side.
>
> This shrinks the size of a Release clang by 16k on x86_64.
>
> Added:
>    llvm/trunk/test/Transforms/SimplifyCFG/phi-undef-loadstore.ll
> Modified:
>    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=138618&r1=138617&r2=138618&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Aug 25 20:22:29 2011
> @@ -2723,6 +2723,72 @@
>   return false;
>  }
>
> +/// Check if passing a value to an instruction will cause undefined behavior.
> +static bool passingValueIsAlwaysUndefined(Value *V, Instruction *I) {
> +  Constant *C = dyn_cast<Constant>(V);
> +  if (!C)
> +    return false;
> +
> +  if (!I->hasOneUse()) // FIXME: There is no reason to limit this to one use.
> +    return false;

I assume this is a performance optimization?

> +  if (C->isNullValue()) {
> +    Instruction *Use = I->use_back();
> +
> +    // Now make sure that there are no instructions in between that can alter
> +    // control flow (eg. calls)
> +    for (BasicBlock::iterator i = ++BasicBlock::iterator(I); &*i != Use; ++i)
> +      if (i == I->getParent()->end() ||
> +          !i->isSafeToSpeculativelyExecute())
> +        return false;

I->hasSideEffects() should be sufficient here... you don't care if
there's an instruction with undefined behavior before the instruction
with undefined behavior. :)

> +    // Look through GEPs. A load from a GEP derived from NULL is still undefined
> +    if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Use))
> +      if (GEP->getPointerOperand() == I)
> +        return passingValueIsAlwaysUndefined(V, GEP);
> +
> +    // Look through bitcasts.
> +    if (BitCastInst *BC = dyn_cast<BitCastInst>(Use))
> +      return passingValueIsAlwaysUndefined(V, BC);
> +
> +    // load from null is undefined
> +    if (isa<LoadInst>(Use))
> +      return true;
> +
> +    // store to null is undef
> +    if (isa<StoreInst>(Use) && Use->getOperand(1) == I)
> +      return true;

Missing address space checks?

-Eli