[llvm-commits] [llvm] r154631 - in /llvm/trunk: include/llvm/Analysis/Dominators.h lib/VMCore/Dominators.cpp

[Duncan Sands]

Hi Dan,

> Add forms of dominates and isReachableFromEntry that accept a Use
> directly instead of a user Instruction. This allows them to test
> whether a def dominates a particular operand if the user instruction
> is a PHI.

how about adding some unittests?  Also, can any code be shared with the other
"dominates" methods?

> --- llvm/trunk/lib/VMCore/Dominators.cpp (original)
> +++ llvm/trunk/lib/VMCore/Dominators.cpp Thu Apr 12 18:31:46 2012
> @@ -184,3 +184,84 @@
>     }
>     return true;
>   }
> +
> +bool DominatorTree::dominates(const Instruction *Def,
> +                              const Use&U) const {
> +  Instruction *UserInst = dyn_cast<Instruction>(U.getUser());
> +
> +  // All non-instructions conceptually dominate everything. Instructions do
> +  // not dominate non-instructions.

While I can see where you are coming from, I can't help feeling that handling
non instructions like this is abusive, and that callers should be required to
explicitly take care of this case.  I.e. that you should assert that UserInst
is not null here.

> +  if (!UserInst)
> +    return !isa<Instruction>(Def);

As Eli mentioned, the return value is always 'false'.

> +  // Ok, def and use are in the same block. If the def is an invoke, it

If the def is an invoke then it was handled already.

> +bool DominatorTree::isReachableFromEntry(const Use&U) const {
> +  Instruction *I = dyn_cast<Instruction>(U.getUser());
> +
> +  // ConstantExprs aren't really reachable from the entry block, but they
> +  // don't need to be treated like unreachable code either.
> +  if (!I) return true;

I think you would do better to assert that I is not null here.

Ciao, Duncan.