Opt Bisect layering

So I’m poking around trying to improve the layering of LLVM (using an internal strict modular build as a motivation/tool to identify issues) & came across a layering violation in the OptBisect implementation.

This feature is implemented in lib/IR, but includes several headers from include/llvm/Analysis - which would create a circular dependency if not for the fact that all the headers it includes are header-only (templates).

Any ideas about how this could/should be fixed? Perhaps these analyses should be moved into somewhere more common - but I don’t know there’s anywhere more (or less) common than lib/IR itself (since I assume they depend on IR & that’s where OptBisect is, so they can’t be lower/higher than that).

Does that sound OK? Anyone have other/better ideas?

  • Dave

It looks like the dependency is really minimal, just an informational query about a Loop/Region/CallGraphSCC and not on any actual analysis. Seems like there should be a pattern that would make that reasonable while maintaining the layering. Maybe a minimalist IRAggregate class that supports what OptBisect needs and becomes a base for the others? (Don’t you love people who do drive-by hand-waving?)

–paulr

There is a patch under review right now from someone who wants to provide a mechanism to replace OptBisect as the pass gate keeping mechanism.

https://reviews.llvm.org/D44464

Possibly we could take this opportunity to move OptBisect to a different layer, though I don’t know where else it would belong.

The other possibility is that we could have the caller pass in a description instead of a pointer to the pass and the IR unit. OptBisect isn’t doing anything with them other than building a string for output.

-Andy