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

[Duncan Sands]

Hi Rafael,

On 19/02/12 21:29, Rafael Ávila de Espíndola wrote:
>> PS: That said, the current logic for handling invokes in dominate does look
>> bogus, so I agree with your diagnosis.
>
> What do you think of the attached patch? I took the liberty of renaming
> the function to make it explicit what it computes.

I think it is still wrong.  What you are forgetting is that it may be possible
to get to the normal destination without passing via the block containing the
invoke.  I suggest you take a look at the logic in the verifier.  I looked at
it pretty carefully long ago, and concluded that it was correct.  I mean this
bit onwards:

       // Check that a definition dominates all of its uses.
       if (InvokeInst *II = dyn_cast<InvokeInst>(Op)) {
         // Invoke results are only usable in the normal destination, not in the
         // exceptional destination.
         BasicBlock *NormalDest = II->getNormalDest();

         Assert2(NormalDest != II->getUnwindDest(),
                 "No uses of invoke possible due to dominance structure!",
                 Op, &I);
...

(Actually it should be possible to slightly simplify this nowadays: it is no
longer legal IR if the unwind and normal destinations are the same).  The meat
of the logic comes after the snippet I've quoted.

By the way, if you move this logic into the dominates routine, then I guess the
verifier can be simplified by removing all the current logic and just replacing
by a call to the improved dominator routine.

Ciao, Duncan.