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

Rafael EspĂ­ndola rafael.espindola at gmail.com
Thu Feb 23 16:35:40 PST 2012


> I was careful to read the full thread this time. Duncan tells us thrice that the normal successor can be a critical edge. But I don't see any example of phis causing a problem. In fact when I saw the code to check phi operands, that's what prompted me to respond. It really shouldn't be necessary. Phi operand dominance is tricky with critical edges (the inverse of the invoke problem). But this interface doesn't deal with operand dominance, only instruction dominance (for all operands)--not tricky.
>
> You can replace your "A dominates pred(phi) for all preds" check with a simple "A dominates parent(phi)".

I was missing phi in general, not just in the case of phi + invoke.


> There are two BBA's above (bad style). They *both* need to dominate BBB. One of us is missing something.

Yes, they both need to dominate, but that is not sufficient. Consider

         InvokeBB
             /\
           /    \
         /       X
        /         \
    EBB  ->   NormalBB

with the use being in NormalBB. In this case the "first BBA" is
InvokeBB,  and it dominates NormalBB. The "second BBA" is NormalBB,
and it dominates itself.

The BB coming from splitting the critical edge (X) doesn't dominate NormalBB.

I have attached updated versions of the patches.

> -Andy

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-the-implementation.patch
Type: text/x-patch
Size: 8835 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120223/3bfe1abc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Use-the-DT-dominate-function-in-the-verifier.patch
Type: text/x-patch
Size: 7546 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120223/3bfe1abc/attachment-0001.bin>


More information about the llvm-commits mailing list