[PATCH] D44464: OptBisect is improved to be overridden in LLVMContext

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added a comment.

In https://reviews.llvm.org/D44464#1042517, @andrew.w.kaylor wrote:

> A few people have suggested that OptBisect could be dropped completely in favor of a debug counter. I'm not sure if that makes supporting your use case easier or harder.


I have seen this suggestion and honestly I dont see how it could be dropped w/o first replacing every invocation of skipPass with a debug-counter's shouldExecute.
Theoretically debug-counters are at least as powerful as OptBisect, but that stays theoretical until somebody really places them into the code.
For now I only see 5 instances of shouldExecute in a whole LLVM tree.

> The current implementation inserts OptBisect into the decision flow by default, but that's almost never what we want.
>  OptBisect does an early return when it isn't enabled, so that's not terrible, but conceptually it's wrong.
>  A single, optional pass gate object that can be set by the client isn't necessarily a bad idea.

So, it seems that Yevgeny's suggestion on optional OptPassGate:

- makes current OptBisect implementation strictly better
- allows us to proceed with our controlling feature
- does not stop anybody from discussing various facets of a better Bisect design

Hence to me it looks like a win-win (or rather win-no-loss ;) situation.

> Ideally, the proposal would start from what's needed for the new pass manager and be sent to the LLVM Dev mailing list for discussion.

I'm personally interested in moving new pass manager forward, so I'ld be glad to participate.
Note, that I'm not particularly tied to/experienced with OptBisect feature, so it would be nice if you could start this discussion.


https://reviews.llvm.org/D44464