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

[Duncan Sands]

Hi Andrew,

>    // If A is an invoke instruction, its value is only available in this normal
>    // successor block.
>    if (const InvokeInst *II = dyn_cast<InvokeInst>(A)) {
>      BBA = II->getNormalDest();
> +   if (BBA == BBB) return true;
>    }

why is this correct?  The normal destination of an invoke could have other
predecessors (in that case the only possible uses of the invoke value are
in phi nodes).  For a routine of this generality, I think it is important
to handle all cases without assuming anything about where the instructions
in question came from (e.g. in Rafael's use case the logic could probably be
simplified, but those special features can't be assumed here, at least not
without auditing all uses of this method and documenting [and checking with
assertions] the assumptions).

By the way, I think the name dominatesUse is a bit confusing.  After all,
instructions always dominates their uses, so it sounds like it should just
return "true" :)  How about leaving the name "dominates" and explaining more
in the comment describing the method (note that the comment in the header
already doesn't match the comment here).  Or just call it:
dominatesInTheAppropriateWayForThisToBeAUseOfTheOther :)

Ciao, Duncan.