[llvm-commits] [llvm] r80920 - in /llvm/trunk: include/llvm/Instructions.h lib/Transforms/Scalar/GVN.cpp lib/Transforms/Utils/BasicBlockUtils.cpp lib/Transforms/Utils/LoopSimplify.cpp lib/Transforms/Utils/PromoteMemoryToRegister.cpp lib/VMCore/Instructions.cpp test/Transforms/GVN/2008-07-02-Unreachable.ll

Chris Lattner clattner at apple.com
Thu Sep 3 09:45:01 PDT 2009


On Sep 3, 2009, at 8:34 AM, Dan Gohman wrote:

> Author: djg
> Date: Thu Sep  3 10:34:35 2009
> New Revision: 80920
>
> URL: http://llvm.org/viewvc/llvm-project?rev=80920&view=rev
> Log:
> Change PHINode::hasConstantValue to have a DominatorTree argument
> instead of a bool argument, and to do the dominator check itself.
> This makes it eaiser to use when DominatorTree information is
> available.

This is a little bit dubious to me because of layering issues.  Going  
forward, I'd really like to move the verifier out of vmcore to  
libanalysis.  This will allows us to move the dominator analysis out  
of vmcore into libanalysis as well, shrinking vmcore.

I agree that this is a dramatically more elegant API for  
hasConstantValue, but can the version that takes dominator info be a  
method somewhere in libanalysis instead of in vmcore?

-Chris

>
> Modified:
>    llvm/trunk/include/llvm/Instructions.h
>    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>    llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp
>    llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp
>    llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
>    llvm/trunk/lib/VMCore/Instructions.cpp
>    llvm/trunk/test/Transforms/GVN/2008-07-02-Unreachable.ll
>
> Modified: llvm/trunk/include/llvm/Instructions.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Instructions.h?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/include/llvm/Instructions.h (original)
> +++ llvm/trunk/include/llvm/Instructions.h Thu Sep  3 10:34:35 2009
> @@ -31,6 +31,7 @@
> class ConstantRange;
> class APInt;
> class LLVMContext;
> +class DominatorTree;
>
> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> //                             AllocationInst Class
> @@ -1950,7 +1951,12 @@
>   /// hasConstantValue - If the specified PHI node always merges  
> together the
>   /// same value, return the value, otherwise return null.
>   ///
> -  Value *hasConstantValue(bool AllowNonDominatingInstruction =  
> false) const;
> +  /// If the PHI has undef operands, but all the rest of the  
> operands are
> +  /// some unique value, return that value if it can be proved that  
> the
> +  /// value dominates the PHI. If DT is null, use a conservative  
> check,
> +  /// otherwise use DT to test for dominance.
> +  ///
> +  Value *hasConstantValue(DominatorTree *DT = 0) const;
>
>   /// Methods for support type inquiry through isa, cast, and  
> dyn_cast:
>   static inline bool classof(const PHINode *) { return true; }
>
> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Sep  3 10:34:35 2009
> @@ -769,7 +769,7 @@
> }
>
> Value* GVN::CollapsePhi(PHINode* p) {
> -  Value* constVal = p->hasConstantValue();
> +  Value* constVal = p->hasConstantValue(DT);
>   if (!constVal) return 0;
>
>   Instruction* inst = dyn_cast<Instruction>(constVal);
>
> Modified: llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp Thu Sep  3  
> 10:34:35 2009
> @@ -429,13 +429,10 @@
>     PN->addIncoming(InVal, NewBB);
>
>     // Check to see if we can eliminate this phi node.
> -    if (Value *V = PN->hasConstantValue(DT != 0)) {
> -      Instruction *I = dyn_cast<Instruction>(V);
> -      if (!I || DT == 0 || DT->dominates(I, PN)) {
> -        PN->replaceAllUsesWith(V);
> -        if (AA) AA->deleteValue(PN);
> -        PN->eraseFromParent();
> -      }
> +    if (Value *V = PN->hasConstantValue(DT)) {
> +      PN->replaceAllUsesWith(V);
> +      if (AA) AA->deleteValue(PN);
> +      PN->eraseFromParent();
>     }
>   }
>
>
> Modified: llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp Thu Sep  3  
> 10:34:35 2009
> @@ -255,7 +255,7 @@
>   PHINode *PN;
>   for (BasicBlock::iterator I = L->getHeader()->begin();
>        (PN = dyn_cast<PHINode>(I++)); )
> -    if (Value *V = PN->hasConstantValue()) {
> +    if (Value *V = PN->hasConstantValue(DT)) {
>       if (AA) AA->deleteValue(PN);
>       PN->replaceAllUsesWith(V);
>       PN->eraseFromParent();
> @@ -417,14 +417,13 @@
>   for (BasicBlock::iterator I = L->getHeader()->begin();  
> isa<PHINode>(I); ) {
>     PHINode *PN = cast<PHINode>(I);
>     ++I;
> -    if (Value *V = PN->hasConstantValue())
> -      if (!isa<Instruction>(V) || DT- 
> >dominates(cast<Instruction>(V), PN)) {
> -        // This is a degenerate PHI already, don't modify it!
> -        PN->replaceAllUsesWith(V);
> -        if (AA) AA->deleteValue(PN);
> -        PN->eraseFromParent();
> -        continue;
> -      }
> +    if (Value *V = PN->hasConstantValue(DT)) {
> +      // This is a degenerate PHI already, don't modify it!
> +      PN->replaceAllUsesWith(V);
> +      if (AA) AA->deleteValue(PN);
> +      PN->eraseFromParent();
> +      continue;
> +    }
>
>     // Scan this PHI node looking for a use of the PHI node by itself.
>     for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
>
> Modified: llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp  
> (original)
> +++ llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp Thu  
> Sep  3 10:34:35 2009
> @@ -494,17 +494,14 @@
>       PHINode *PN = I->second;
>
>       // If this PHI node merges one value and/or undefs, get the  
> value.
> -      if (Value *V = PN->hasConstantValue(true)) {
> -        if (!isa<Instruction>(V) ||
> -            properlyDominates(cast<Instruction>(V), PN)) {
> -          if (AST && isa<PointerType>(PN->getType()))
> -            AST->deleteValue(PN);
> -          PN->replaceAllUsesWith(V);
> -          PN->eraseFromParent();
> -          NewPhiNodes.erase(I++);
> -          EliminatedAPHI = true;
> -          continue;
> -        }
> +      if (Value *V = PN->hasConstantValue(&DT)) {
> +        if (AST && isa<PointerType>(PN->getType()))
> +          AST->deleteValue(PN);
> +        PN->replaceAllUsesWith(V);
> +        PN->eraseFromParent();
> +        NewPhiNodes.erase(I++);
> +        EliminatedAPHI = true;
> +        continue;
>       }
>       ++I;
>     }
>
> Modified: llvm/trunk/lib/VMCore/Instructions.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Instructions.cpp?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/VMCore/Instructions.cpp (original)
> +++ llvm/trunk/lib/VMCore/Instructions.cpp Thu Sep  3 10:34:35 2009
> @@ -17,6 +17,7 @@
> #include "llvm/Function.h"
> #include "llvm/Instructions.h"
> #include "llvm/Operator.h"
> +#include "llvm/Analysis/Dominators.h"
> #include "llvm/Support/ErrorHandling.h"
> #include "llvm/Support/CallSite.h"
> #include "llvm/Support/ConstantRange.h"
> @@ -226,7 +227,12 @@
> /// hasConstantValue - If the specified PHI node always merges  
> together the same
> /// value, return the value, otherwise return null.
> ///
> -Value *PHINode::hasConstantValue(bool  
> AllowNonDominatingInstruction) const {
> +/// If the PHI has undef operands, but all the rest of the operands  
> are
> +/// some unique value, return that value if it can be proved that the
> +/// value dominates the PHI. If DT is null, use a conservative check,
> +/// otherwise use DT to test for dominance.
> +///
> +Value *PHINode::hasConstantValue(DominatorTree *DT) const {
>   // If the PHI node only has one incoming value, eliminate the PHI  
> node...
>   if (getNumIncomingValues() == 1) {
>     if (getIncomingValue(0) != this)   // not  X = phi X
> @@ -260,12 +266,19 @@
>   // instruction, we cannot always return X as the result of the PHI  
> node.  Only
>   // do this if X is not an instruction (thus it must dominate the  
> PHI block),
>   // or if the client is prepared to deal with this possibility.
> -  if (HasUndefInput && !AllowNonDominatingInstruction)
> -    if (Instruction *IV = dyn_cast<Instruction>(InVal))
> -      // If it's in the entry block, it dominates everything.
> -      if (IV->getParent() != &IV->getParent()->getParent()- 
> >getEntryBlock() ||
> -          isa<InvokeInst>(IV))
> -        return 0;   // Cannot guarantee that InVal dominates this  
> PHINode.
> +  if (HasUndefInput)
> +    if (Instruction *IV = dyn_cast<Instruction>(InVal)) {
> +      if (DT) {
> +        // We have a DominatorTree. Do a precise test.
> +        if (!DT->dominates(IV, this))
> +          return 0;
> +      } else {
> +        // If it's in the entry block, it dominates everything.
> +        if (IV->getParent() != &IV->getParent()->getParent()- 
> >getEntryBlock() ||
> +            isa<InvokeInst>(IV))
> +          return 0;   // Cannot guarantee that InVal dominates this  
> PHINode.
> +      }
> +    }
>
>   // All of the incoming values are the same, return the value now.
>   return InVal;
>
> Modified: llvm/trunk/test/Transforms/GVN/2008-07-02-Unreachable.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/2008-07-02-Unreachable.ll?rev=80920&r1=80919&r2=80920&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/test/Transforms/GVN/2008-07-02-Unreachable.ll  
> (original)
> +++ llvm/trunk/test/Transforms/GVN/2008-07-02-Unreachable.ll Thu  
> Sep  3 10:34:35 2009
> @@ -1,4 +1,4 @@
> -; RUN: llvm-as < %s | opt -gvn | llvm-dis | grep undef
> +; RUN: llvm-as < %s | opt -gvn | llvm-dis | grep {ret i8 \[%\]tmp3}
> ; PR2503
>
> @g_3 = external global i8		; <i8*> [#uses=2]
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list