[PATCH] D130952: [Inliner] Handle multiple `run` invocation of `ModuleInlinerWrapperPass` (NFC).

Thu Aug 4 11:01:37 PDT 2022

mtrofin added a comment.

In D130952#3698544 <https://reviews.llvm.org/D130952#3698544>, @michele.scandale wrote:

>> Wondering, is this something that's intentionally supported though?
>
> Based on the discussion in D130954 <https://reviews.llvm.org/D130954>, it seems that currently there is no intention to support the use case I was experimenting with, therefore this change is not required.
> From the initial performance analysis rebuilding the pass manager is much cheaper with the new pass manager than the legacy one, and for now I can use such alternative approach for my use case.
>
>> Setting that question aside, maybe it'd make more sense we moved the finalization stuff in a `ModuleInlinerWrapperPass::finish()` or something like that, and then we just `assert(Finalized)` in `::run`; that would address a current oddity with the wrapper, in that the pass composition is somewhat deferred until the run.
>>
>> wdyt?
>
> Clients of `ModuleInlinerWrapperPass` would need  to call `finish` before using it. I don't know if that acceptable or not as there is no `finish` for regular pass managers.
> Perhaps it would be better to find a way to avoid the pass managers moves inside `run`. It seems that explicitly allocating the `PassModel` for a `CGSCCPassManager` would allow to keep a pointer to the `CGSCCPassManager` that would be returned via `getPM`, and still be able to prepare the module pass manager with the `ModuleToPostOrderCGSCCPassAdaptor` directly in the constructor of `ModuleInlinerWrapperPass`.

Right, so then the lines 1160-1166 would be the responsibility of the user, and the main responsibility of the ModuleInlinerWrapperPass is to set up / clear out correctly the advisor. SGTM.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130952/new/

https://reviews.llvm.org/D130952