[PATCH] D36553: [InstCombine] Add a DEBUG_COUNTER to InstCombine to limit how many instructions are visited for debug

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

In https://reviews.llvm.org/D36553#837752, @dberlin wrote:

> > The debug counter infrastructure also supports skipping some number of calls at the beginning as well, but that feels like it would generate very odd behavior with InstCombine.
>
>
>
> > 
>
> FWIW: I think i would say that when it can be done reasonably, it can help a lot on large files. But it is definitely difficult, as we implement some optimizations, to have it work in a fashion that makes a ton of sense to think about.  IE it's easy if the optimization just has candidates that it processes once.
>
> For NewGVN, we go to the trouble of marking things such that it acts like a candidate counter (
>  IE if we skip value numbering A the first time, we'll always skip it) , because it can help us a lot to narrow down the space of stuff getting value numbered, and in some cases, reduce testcases to value numbering a single def-use chain.
>
> All that said, since the goal is faster reduction and debugging, sometimes not thinking about it works too. Most optimization should *not* crash or break just because you decide not to optimize the first n instructions or whatever.
>  In this case,   InstCombine shouldn't crash or break just because we randomly decide not to optimize the first N things on the worklist.


Yes. I'd say all these are bugs.
Whether we care or not, it's a different story (although I think we should because instcombine could change in a way that exposes them).
Also, Unfortunately is not uncommon to find a (even worse problem), which is, trying to reduce a testcase with opt-bisect triggers crashes in the backend (as we stop earlier in the pipeline).
Zhendong's fuzzer exposes a lot of them (as it generates bitcode and runs somehow arbitrary pipelines to trigger crashes).

tl;dr Craig, I agree that I think you may want to make that change and then we should deal with the fallout :)


https://reviews.llvm.org/D36553