[PATCH] D47858: [New PM] Introducing PassInstrumentation framework

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev marked 4 inline comments as done.
fedor.sergeev added a comment.

In https://reviews.llvm.org/D47858#1217605, @philip.pfaffe wrote:

> Back from vacation, and had time for a closer look.
>
> My major concern right now is ownership.


Yes, this is one of the main concerns for me as well.

> In your current state, `PassBuilder` owns both all the callbacks as well as the state of the instrumentation (i.e., the counter). As it were, PassBuilder is in fact more of a transient factory that you can throw a way after you've set up your pipeline. Or you might even use it to make more than one pipeline. In both cases ownership is a problem.

This is exactly when submitting my RFC I was going to use LLVMContext for this ownership.
Some of replies (including yours ;) - http://lists.llvm.org/pipermail/llvm-dev/2018-June/123850.html) were in favor of PassBuilder.
PassBuilder is not ideal for my own downstream project either - we do not keep PassBuilder around.
But  then we need to find something else that owns.

> Instead, I believe it makes for nicer separation if PassBuilder becomes purely the entry point for callback registration, and all bookkeeping happens within the pipeline. IMO,  a good place to put this would be the Module-level Analysis itself, which is a singleton wrt. a single compilation.

Ehm... let me try this. When I quick-checked it last time I did not like amount of changes I need to do for that. But it is doable and I'm definitely more familiar with codebase as of right now.

> Federation between different IRUnits can happen via proxies, which adds some amount of indirections, but I still think that's manageable.

I'm not particularly happy with all these PassInstrumentationAnalysis runs, for one because it makes updating all the new-pass-manager tests that use -debug-pass-manger quite a churn. :)

> Lastly, I think this deserves unittests. You can check out PassBuilderCallbacksTests or LoopPassManagerTests for inspiration on how to mock out passes and analyses.

Definitely. I just had no experience with llvm unit-tests before that, so I decided to postpone doing them until after we settle on main design points.


Repository:
  rL LLVM

https://reviews.llvm.org/D47858