[PATCH] D47858: [New PM] Introducing PassInstrumentation framework
Fedor Sergeev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 29 09:58:32 PDT 2018
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
More information about the llvm-commits
mailing list