[PATCH] D34135: [LVI] Add initial result to avoid infinite getValueFromCondition recursion

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

In https://reviews.llvm.org/D34135#781243, @anna wrote:

> In https://reviews.llvm.org/D34135#780695, @dberlin wrote:
>
> > In https://reviews.llvm.org/D34135#780666, @anna wrote:
> >
> > > In https://reviews.llvm.org/D34135#780637, @efriedma wrote:
> > >
> > > > The sequence here is that we constant-fold a terminator (deleting an edge), then continue iterating through all the blocks in the function, and eventually crash when LVI discovers a cycle in a loop which was made unreachable.
> > >
> > >
> > > So, the main problem here is that in each iteration of Jump threading, we can find the trivially dead blocks, but not the complete set of unreachable blocks. Also, I'm assuming transitively detecting dead blocks after the call to `processBlock` does not help in this case?
> >
> >
> > The set of blocks it will find will depend on input order of basic blocks currently.
> >  It is possible to increase it to find all non-cycle involved unreachable blocks  by performing the processing in the optimal iteration order.
> >  It should be possible to find all unreachable blocks (including those involved in cycle) as you go by forming SCC's and iterating them individually in the right order.
>
>
> Agreed. Also, there was even a discussion on changing the ordering for JumpThreading because of how the current ordering can cause certain blocks to be dead, and AA chokes on the PHIs in those blocks: https://bugs.llvm.org/show_bug.cgi?id=33142#c14
>  (This might be completely irrelevant for this patch, but it's around the same problem of identifying  unreachability at all times in JumpThreading).


Yes. As the person who did the analysis on that bug I decided to not go that direction (i.e. use a stack worklist) because I wasn't able to prove myself it was correct.
I've seen at least another example internally where JumpThreading goes bananas because of bad iteration ordering. Iterating SCC's topologically will solve at least part of  these problems,
I think, but I'm afraid that's a much larger change. I wouldn't stop this from going in but at the same time it's likely Zhendong's fuzzer (or other fuzzers, or simply production code :) will uncover similar issues to this.

i.e. IMHO the long term solution is to rewrite JT but nobody seems to have the bandwidth to do it for now.


https://reviews.llvm.org/D34135