[llvm-commits] [llvm] r150885 - /llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp

[Duncan Sands]

Hi Rafael,

>> Looks like the problem was not the assert, but the dominate function!
...
> --- a/lib/VMCore/Dominators.cpp
> +++ b/lib/VMCore/Dominators.cpp
> @@ -83,15 +83,25 @@ void DominatorTree::print(raw_ostream &OS, const Module *) const {
>  // dominates - Return true if A dominates a use in B. This performs the

note what the comment says: if A dominates *a use* in B.  An instruction can't
use itself (in a reachable basic block), so the A == B check is wrong.

>  // special checks necessary if A and B are in the same basic block.
>  bool DominatorTree::dominates(const Instruction *A, const Instruction *B) const{
> +  // This is needed to handle A and B being the same invoke.
> +  if (A == B)
> +    return true;

This seems wrong, see above.

> +
>    const BasicBlock *BBA = A->getParent(), *BBB = B->getParent();
>
>    // If A is an invoke instruction, its value is only available in this normal
>    // successor block.
> -  if (const InvokeInst *II = dyn_cast<InvokeInst>(A))
> +  const InvokeInst *II = dyn_cast<InvokeInst>(A);
> +  if (II)
>      BBA = II->getNormalDest();
>
>    if (BBA != BBB) return dominates(BBA, BBB);
>
> +  // If A is an invoke, it dominates anything on the normal successor block.
> +  // FIXME: Should it dominate phis?
> +  if (II)
> +    return true;

This seems also wrong.  The normal successor block can have many predecessors
in general.

Ciao, Duncan.

> +
>    // It is not possible to determine dominance between two PHI nodes
>    // based on their ordering.
>    if (isa<PHINode>(A) && isa<PHINode>(B))