[llvm][inliner] Reuse the inliner pass to implement 'always inliner'

[mtrofin]

Summary

Enable performing mandatory inlinings upfront, by reusing the same logic
as the full inliner, instead of the AlwaysInliner. This has the
following benefits:

• reduce code duplication - one inliner codebase
• open the opportunity to help the full inliner by performing additional

function passes after the mandatory inlinings, but before th full
inliner. Performing the mandatory inlinings first simplifies the problem
the full inliner needs to solve: less call sites, more contextualization, and,
depending on the additional function optimization passes run between the
2 inliners, higher accuracy of cost models / decision policies.

Note that this patch does not yet enable much in terms of post-always
inline function optimization.

[mtrofin]

Please note: the patch isn't 100% ready, there are those tests that check how the pipeline is composed, which are unpleasant to fix, so I want to defer them to after we get agreement over the larger points this patch brings (i.e. pre-performing always inlinings, value in further exploring cleanups before full inlining, etc)

[dblaikie]

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

[mtrofin]

In D91567#2398440, @dblaikie wrote:

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

In D91567#2398440, @dblaikie wrote:

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

[dblaikie]

In D91567#2398461, @mtrofin wrote:

In D91567#2398440, @dblaikie wrote:

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

OK, so if I understand correctly with the old Pass Manager there were two separate passes (always inliner and inliner - they share some code though, yeah?) and they were run in the pass pipeline but potentially (definitely?) not adjacent? New pass manager survived for quite a while with only one inlining pass, that included a mandatorily strong preference for inlining always-inline functions? But still missed some recursive cases. So D86988 made the always inliner run right next to/before the inliner in the NPM.

Now there's tihs patch, to implement the AlwaysInliner using the inliner - but is also changing the order of passes to improve optimization opportunities by doing some cleanup after always inlining?

[mtrofin]

In D91567#2398623, @dblaikie wrote:

In D91567#2398461, @mtrofin wrote:

In D91567#2398440, @dblaikie wrote:

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

OK, so if I understand correctly with the old Pass Manager there were two separate passes (always inliner and inliner - they share some code though, yeah?)

AlwaysInlinerLegacyPass does, yes. The NPM variant doesn't.

and they were run in the pass pipeline but potentially (definitely?) not adjacent?

From what I can see, the legacy one was used only in the O0/O1 cases, see clang/lib/CodeGen/BackendUtil,cpp:643. The full inliner isn't.

New pass manager survived for quite a while with only one inlining pass, that included a mandatorily strong preference for inlining always-inline functions? But still missed some recursive cases. So D86988 made the always inliner run right next to/before the inliner in the NPM.

Now there's tihs patch, to implement the AlwaysInliner using the inliner - but is also changing the order of passes to improve optimization opportunities by doing some cleanup after always inlining?

It doesn't quite change the order D86988 introduced. Specifically, D86988 ran AlwaysInliner (a module pass) first, then let the Inliner and function optimizations happen.
This patch keeps the order between doing mandatory inlinings and inlinings. But, in addition, if in the future we want to also perform some of the function passes that happen in the inliner case, to help the full inliner, we can more easily do so.

[aeubanks]

What about removing the existing AlwaysInlinerPass and replacing it with this one? Or is that something you were planning to do in a follow-up change?

open the opportunity to help the full inliner by performing additional function passes after the mandatory inlinings, but before the full inliner

This change doesn't run the function simplification pipeline between the mandatory and full inliner though, only

if (AttributorRun & AttributorRunOption::CGSCC)
  MainCGPipeline.addPass(AttributorCGSCCPass());

if (PTO.Coroutines)
  MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));

// Now deduce any function attributes based in the current code.
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());

And is there any evidence that running the function simplification pipeline between the mandatory and full inliner is helpful? It could affect compile times.

I'd think that adding the mandatory inliner right before the full inliner in the same CGSCC pass manager would do the job. e.g. add it in ModuleInlinerWrapperPass::ModuleInlinerWrapperPass() right before PM.addPass(InlinerPass());

llvm/include/llvm/Analysis/InlineAdvisor.h

27

4

[mtrofin]

In D91567#2400207, @aeubanks wrote:

What about removing the existing AlwaysInlinerPass and replacing it with this one? Or is that something you were planning to do in a follow-up change?

That's the plan, yes.

open the opportunity to help the full inliner by performing additional function passes after the mandatory inlinings, but before the full inliner

This change doesn't run the function simplification pipeline between the mandatory and full inliner though, only

if (AttributorRun & AttributorRunOption::CGSCC)
  MainCGPipeline.addPass(AttributorCGSCCPass());

if (PTO.Coroutines)
  MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));

// Now deduce any function attributes based in the current code.
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());

Right - my point was that we could more easily explore doing so by having this new AlwaysInliner separate. In this patch I included those additional passes because I thought they may be necessary or beneficial, but I can remove them as a first step if they are not necessary. @jdoerfert - should the Attributor be run post-always inlining, or is it fine to not be run?

And is there any evidence that running the function simplification pipeline between the mandatory and full inliner is helpful? It could affect compile times.

In the ML-driven -Oz case, we saw some marginal improvement. I haven't in -O3 cases using the "non-ml" inliner. I suspect that it helps in the ML policy case (both -Oz and -O3, on which we're currently working), because: 1) the current -Oz takes some global (module-wide) features, so probably simplifying out the trivial cases helps; and 2) we plan on taking into consideration regions of the call graph, and the intuition is that eliminating the trivial cases (mandatory cases) would rise the visibility (for a training algorithm) of the non-trivial cases.

I'd think that adding the mandatory inliner right before the full inliner in the same CGSCC pass manager would do the job. e.g. add it in ModuleInlinerWrapperPass::ModuleInlinerWrapperPass() right before PM.addPass(InlinerPass());

It would, and what I'm proposing here is equivalent to that, but the proposal here helps with these other explorations, with (arguably) not much of a difference cost-wise in itself (meaning, of course, if we discover there's benefit in running those additional passes, we pay with compile time, but in of itself, factoring the always inliner in its own wrapper, or in the same wrapper as the inliner, doesn't really come at much of a cost).

Now, if we determine there is no value, we can bring it back easily - wdyt?

[dblaikie]

In D91567#2398637, @mtrofin wrote:

In D91567#2398623, @dblaikie wrote:

In D91567#2398461, @mtrofin wrote:

In D91567#2398440, @dblaikie wrote:

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

OK, so if I understand correctly with the old Pass Manager there were two separate passes (always inliner and inliner - they share some code though, yeah?)

AlwaysInlinerLegacyPass does, yes. The NPM variant doesn't.

The NPM always inliner doesn't share any code with the NPM non-always inliner? (though this ( https://reviews.llvm.org/D86988 ) is the patch that added a separate always inliner to the NPM, right? And that patch doesn't look like it adds a whole new pass implementation - so looks like it's sharing some code with something?)

and they were run in the pass pipeline but potentially (definitely?) not adjacent?

From what I can see, the legacy one was used only in the O0/O1 cases, see clang/lib/CodeGen/BackendUtil,cpp:643. The full inliner isn't.

The full inliner isn't.. isn't run at -O0/-O1? So with the Legacy Pass Manager one inliner (always or non-always) was used in a given compilation, not both? (so I guess then the non-always inliner did the always-inlining in -O2 and above in the old pass manager? But didn't have the same recursive always inlining miss that the NPM non-always inliner had?)

New pass manager survived for quite a while with only one inlining pass, that included a mandatorily strong preference for inlining always-inline functions? But still missed some recursive cases. So D86988 made the always inliner run right next to/before the inliner in the NPM.

Now there's tihs patch, to implement the AlwaysInliner using the inliner - but is also changing the order of passes to improve optimization opportunities by doing some cleanup after always inlining?

It doesn't quite change the order D86988 introduced. Specifically, D86988 ran AlwaysInliner (a module pass) first, then let the Inliner and function optimizations happen.
This patch keeps the order between doing mandatory inlinings and inlinings. But, in addition, if in the future we want to also perform some of the function passes that happen in the inliner case, to help the full inliner, we can more easily do so.

I'm still a bit confused/trying to understand better - am I understanding correctly when I say: D86988 added always inlining (for the NPM) as a separate process within the non-always inliner? And this patch you're proposing. breaks always inlining out into a separate pass proper, so that at some point, if someone wanted to (but not being done in this patch) they could put some passes in between the two runs of inlining (always and non-always)?

(I guess one thing I might be especially confused about is the "reuse X to do Y" would, to me, immediately lead me to think about "so I expect to see a bunch of deleted code because X presumably was doing a bunch of stuff itself that it now doesn't have to" - but I guess that's not the case here? (at least I don't see a large bunch of deletion I'd expect to see if some kind of inlining implementation was being deleted))

[mtrofin]

In D91567#2401021, @dblaikie wrote:

In D91567#2398637, @mtrofin wrote:

In D91567#2398623, @dblaikie wrote:

In D91567#2398461, @mtrofin wrote:

In D91567#2398440, @dblaikie wrote:

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

OK, so if I understand correctly with the old Pass Manager there were two separate passes (always inliner and inliner - they share some code though, yeah?)

AlwaysInlinerLegacyPass does, yes. The NPM variant doesn't.

The NPM always inliner doesn't share any code with the NPM non-always inliner? (though this ( https://reviews.llvm.org/D86988 ) is the patch that added a separate always inliner to the NPM, right? And that patch doesn't look like it adds a whole new pass implementation - so looks like it's sharing some code with something?)

There was already an AlwaysInliner for the NPM, just wasn't used. So D86966 hooked that up in the NPM, basically. The implementation of that AlwaysInliner is separate from the Inliner pass. See Transforms/IPO/AlwaysInliner.cpp lines 36 - 114, vs Inliner.cpp, from 687 onwards.

and they were run in the pass pipeline but potentially (definitely?) not adjacent?

From what I can see, the legacy one was used only in the O0/O1 cases, see clang/lib/CodeGen/BackendUtil,cpp:643. The full inliner isn't.

The full inliner isn't.. isn't run at -O0/-O1? So with the Legacy Pass Manager one inliner (always or non-always) was used in a given compilation, not both? (so I guess then the non-always inliner did the always-inlining in -O2 and above in the old pass manager? But didn't have the same recursive always inlining miss that the NPM non-always inliner had?)

Yup, see BackendUtil.cpp:634. Can't comment on the latter problem.

New pass manager survived for quite a while with only one inlining pass, that included a mandatorily strong preference for inlining always-inline functions? But still missed some recursive cases. So D86988 made the always inliner run right next to/before the inliner in the NPM.

Now there's tihs patch, to implement the AlwaysInliner using the inliner - but is also changing the order of passes to improve optimization opportunities by doing some cleanup after always inlining?

It doesn't quite change the order D86988 introduced. Specifically, D86988 ran AlwaysInliner (a module pass) first, then let the Inliner and function optimizations happen.
This patch keeps the order between doing mandatory inlinings and inlinings. But, in addition, if in the future we want to also perform some of the function passes that happen in the inliner case, to help the full inliner, we can more easily do so.

I'm still a bit confused/trying to understand better - am I understanding correctly when I say: D86988 added always inlining (for the NPM) as a separate process within the non-always inliner? And this patch you're proposing. breaks always inlining out into a separate pass proper, so that at some point, if someone wanted to (but not being done in this patch) they could put some passes in between the two runs of inlining (always and non-always)?

Yes. Nit on the first sentence: it's not "a separate process *within* the non-always inliner". It's a separate module pass part of the module pass manager that wraps the full inliner and related passes.

(I guess one thing I might be especially confused about is the "reuse X to do Y" would, to me, immediately lead me to think about "so I expect to see a bunch of deleted code because X presumably was doing a bunch of stuff itself that it now doesn't have to" - but I guess that's not the case here? (at least I don't see a large bunch of deletion I'd expect to see if some kind of inlining implementation was being deleted))

See the note to @aeubanks' - indeed, we can remove the NPM AlwaysInliner as result of this change.

[dblaikie]

Thanks for the walkthroughs/help. Also stared at the code a bit. I think I get it now. Some of the confusion also came from having both LPM and NPM versions of the always inliner in the same file, though they seem to share no code.

I'll leave the more nuanced review to folks more familiar with it - sorry for any noise.

[aeubanks]

In D91567#2400699, @mtrofin wrote:

In D91567#2400207, @aeubanks wrote:

What about removing the existing AlwaysInlinerPass and replacing it with this one? Or is that something you were planning to do in a follow-up change?

That's the plan, yes.

open the opportunity to help the full inliner by performing additional function passes after the mandatory inlinings, but before the full inliner

This change doesn't run the function simplification pipeline between the mandatory and full inliner though, only

if (AttributorRun & AttributorRunOption::CGSCC)
  MainCGPipeline.addPass(AttributorCGSCCPass());

if (PTO.Coroutines)
  MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));

// Now deduce any function attributes based in the current code.
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());

Right - my point was that we could more easily explore doing so by having this new AlwaysInliner separate. In this patch I included those additional passes because I thought they may be necessary or beneficial, but I can remove them as a first step if they are not necessary. @jdoerfert - should the Attributor be run post-always inlining, or is it fine to not be run?

I'd say start off without running any passes and keeping the status quo (i.e. an NFC patch), then explore adding passes between the inliners in a future patch.

And is there any evidence that running the function simplification pipeline between the mandatory and full inliner is helpful? It could affect compile times.

In the ML-driven -Oz case, we saw some marginal improvement. I haven't in -O3 cases using the "non-ml" inliner. I suspect that it helps in the ML policy case (both -Oz and -O3, on which we're currently working), because: 1) the current -Oz takes some global (module-wide) features, so probably simplifying out the trivial cases helps; and 2) we plan on taking into consideration regions of the call graph, and the intuition is that eliminating the trivial cases (mandatory cases) would rise the visibility (for a training algorithm) of the non-trivial cases.

I'd think that adding the mandatory inliner right before the full inliner in the same CGSCC pass manager would do the job. e.g. add it in ModuleInlinerWrapperPass::ModuleInlinerWrapperPass() right before PM.addPass(InlinerPass());

It would, and what I'm proposing here is equivalent to that, but the proposal here helps with these other explorations, with (arguably) not much of a difference cost-wise in itself (meaning, of course, if we discover there's benefit in running those additional passes, we pay with compile time, but in of itself, factoring the always inliner in its own wrapper, or in the same wrapper as the inliner, doesn't really come at much of a cost).

Now, if we determine there is no value, we can bring it back easily - wdyt?

I'll run this through llvm-compile-time-tracker to see what the compile time implications are.

[mtrofin]

Running just the always inliner variant, without other passes.

[google-llvm-upstream-contributions]

This comment was removed by google-llvm-upstream-contributions.

[aeubanks]

I'll run this through llvm-compile-time-tracker to see what the compile time implications are.

You mean for the variant where we ran some of the function passes, or you'd try running all of them? Probably the latter would be quite interesting as a 'worst case'.

I was trying the previous patch, but will also try running all function passes, definitely would be interesting.

[mtrofin]

In D91567#2403173, @aeubanks wrote:

In D91567#2400699, @mtrofin wrote:

In D91567#2400207, @aeubanks wrote:

What about removing the existing AlwaysInlinerPass and replacing it with this one? Or is that something you were planning to do in a follow-up change?

That's the plan, yes.

open the opportunity to help the full inliner by performing additional function passes after the mandatory inlinings, but before the full inliner

This change doesn't run the function simplification pipeline between the mandatory and full inliner though, only

if (AttributorRun & AttributorRunOption::CGSCC)
  MainCGPipeline.addPass(AttributorCGSCCPass());

if (PTO.Coroutines)
  MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));

// Now deduce any function attributes based in the current code.
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());

Right - my point was that we could more easily explore doing so by having this new AlwaysInliner separate. In this patch I included those additional passes because I thought they may be necessary or beneficial, but I can remove them as a first step if they are not necessary. @jdoerfert - should the Attributor be run post-always inlining, or is it fine to not be run?

I'd say start off without running any passes and keeping the status quo (i.e. an NFC patch), then explore adding passes between the inliners in a future patch.

Done

And is there any evidence that running the function simplification pipeline between the mandatory and full inliner is helpful? It could affect compile times.

In the ML-driven -Oz case, we saw some marginal improvement. I haven't in -O3 cases using the "non-ml" inliner. I suspect that it helps in the ML policy case (both -Oz and -O3, on which we're currently working), because: 1) the current -Oz takes some global (module-wide) features, so probably simplifying out the trivial cases helps; and 2) we plan on taking into consideration regions of the call graph, and the intuition is that eliminating the trivial cases (mandatory cases) would rise the visibility (for a training algorithm) of the non-trivial cases.

I'd think that adding the mandatory inliner right before the full inliner in the same CGSCC pass manager would do the job. e.g. add it in ModuleInlinerWrapperPass::ModuleInlinerWrapperPass() right before PM.addPass(InlinerPass());

It would, and what I'm proposing here is equivalent to that, but the proposal here helps with these other explorations, with (arguably) not much of a difference cost-wise in itself (meaning, of course, if we discover there's benefit in running those additional passes, we pay with compile time, but in of itself, factoring the always inliner in its own wrapper, or in the same wrapper as the inliner, doesn't really come at much of a cost).

Now, if we determine there is no value, we can bring it back easily - wdyt?

I'll run this through llvm-compile-time-tracker to see what the compile time implications are.

You mean for the variant where we ran some of the function passes, or you'd try running all of them? Probably the latter would be quite interesting as a 'worst case'.

[mtrofin]

In D91567#2403216, @aeubanks wrote:

I'll run this through llvm-compile-time-tracker to see what the compile time implications are.

You mean for the variant where we ran some of the function passes, or you'd try running all of them? Probably the latter would be quite interesting as a 'worst case'.

I was trying the previous patch, but will also try running all function passes, definitely would be interesting.

Awesome! Thanks!

If the rest of the (now NFC) patch seems reasonable, I'll go through those pesky pass manager tests to finish it up.

[aeubanks]

One thing that would be nice would be to have both inliners in the same CGSCC pass manager to avoid doing SCC construction twice, but that would require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a future cleanup.

[mtrofin]

In D91567#2403236, @aeubanks wrote:

One thing that would be nice would be to have both inliners in the same CGSCC pass manager to avoid doing SCC construction twice, but that would require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a future cleanup.

There's that benefit to simplifying the module with the always inliner before doing inlining "in earnest" I was pointing earlier at: for the ML policies work, we plan on capturing (sub)graph information. Using the same SCC would not help because the "higher" (callers) parts of the graph would have these mandatory inlinings not completed yet, and thus offer a less accurate picture of the problem space.

[aeubanks]

In D91567#2403252, @mtrofin wrote:

In D91567#2403236, @aeubanks wrote:

One thing that would be nice would be to have both inliners in the same CGSCC pass manager to avoid doing SCC construction twice, but that would require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a future cleanup.

There's that benefit to simplifying the module with the always inliner before doing inlining "in earnest" I was pointing earlier at: for the ML policies work, we plan on capturing (sub)graph information. Using the same SCC would not help because the "higher" (callers) parts of the graph would have these mandatory inlinings not completed yet, and thus offer a less accurate picture of the problem space.

Oh I see, caller information is useful.

For compile times: http://llvm-compile-time-tracker.com/?config=O3&stat=instructions&remote=aeubanks.
The previous version of this patch (perf/npmalways) running a couple passes has some small but measurable overhead on some benchmarks, 0.5%.
The version of running everything (perf/npmalways2) hugely increases compile times, almost by 50% in one case.

[mtrofin]

In D91567#2403544, @aeubanks wrote:

In D91567#2403252, @mtrofin wrote:

In D91567#2403236, @aeubanks wrote:

One thing that would be nice would be to have both inliners in the same CGSCC pass manager to avoid doing SCC construction twice, but that would require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a future cleanup.

There's that benefit to simplifying the module with the always inliner before doing inlining "in earnest" I was pointing earlier at: for the ML policies work, we plan on capturing (sub)graph information. Using the same SCC would not help because the "higher" (callers) parts of the graph would have these mandatory inlinings not completed yet, and thus offer a less accurate picture of the problem space.

Oh I see, caller information is useful.

For compile times: http://llvm-compile-time-tracker.com/?config=O3&stat=instructions&remote=aeubanks.
The previous version of this patch (perf/npmalways) running a couple passes has some small but measurable overhead on some benchmarks, 0.5%.
The version of running everything (perf/npmalways2) hugely increases compile times, almost by 50% in one case.

Thanks for doing this! Really good to have this data.

[mtrofin]

patched up tests

[tejohnson]

From a ThinLTO perspective, no specific concerns as the buildModuleSimplificationPipeline is invoked in both the pre and post LTO link pipelines, so they both get an equivalent change. But there is an issue for regular LTO, noted below.

llvm/test/Other/new-pm-lto-defaults.ll

70 ↗

(On Diff #306593)

Note there is no corresponding add of an additional InlinerPass like in the other files. The reason is that PassBuilder::buildLTODefaultPipeline doesn't invoke buildModuleSimplificationPipeline, or even buildInlinerPipeline (it has a separate pipeline setup for compile time reasons due to the monolithic nature of the post-LTO link compilation), but rather directly adds ModuleInlinerWrapperPass. So you'll want to add the additional ModuleInlinerWrapperPass invocation there as well.

[mtrofin]

Fixed the LTO case.

Also fixed the p46945 test, which, post - D90566, was passing without the need of a preliminary always-inlier pass.
The reason is that the order of the traversal of the functions in a SCC changed. The test requies that the 'alwaysinline'
function be processed first (to render it recursive and, thus, uninlinable).

[aeubanks]

aside from some nits, lgtm
thanks for doing this!

clang/test/Frontend/optimization-remark-line-directive.c

5

the change on this line shouldn't be necessary, this is a legacy PM RUN line

llvm/include/llvm/Analysis/InlineAdvisor.h

27

ping

llvm/test/Transforms/Inline/pr46945.ll

1–2 ↗

(On Diff #308422)

maybe we should have a RUN line with -passes='default<O2>' to make sure the whole thing works

[mtrofin]

fixes

llvm/include/llvm/Analysis/InlineAdvisor.h

27

sorry - done

clang/test/Frontend/optimization-remark-line-directive.c

 // This file tests -Rpass diagnostics together with #line
 // directives. We cannot map #line directives back to
 // a SourceLocation.
 
-// RUN: %clang_cc1 %s -Rpass=inline -debug-info-kind=line-tables-only -emit-llvm-only -verify -fno-experimental-new-pass-manager
+// RUN: %clang_cc1 %s -Rpass=inline -debug-info-kind=line-tables-only -emit-llvm-only -verify -fno-experimental-new-pass-manager -mllvm -mandatory-inlining-first=0
 
 // The new PM inliner is not added to the default pipeline at O0, so we add
 // some optimizations to trigger it.
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 -debug-info-kind=line-tables-only -emit-llvm-only -verify
+// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 -debug-info-kind=line-tables-only -emit-llvm-only -verify -mllvm -mandatory-inlining-first=0
 
 int foo(int x, int y) __attribute__((always_inline));
 int foo(int x, int y) { return x + y; }
 
 // expected-remark@+2 {{foo inlined into bar}} expected-note@+2 {{could not determine the original source location for /bad/path/to/original.c:1230:25}}
 #line 1230 "/bad/path/to/original.c"
 int bar(int j) { return foo(j, j - 2); }

llvm/include/llvm/Analysis/InlineAdvisor.h

 
 namespace llvm {
 class BasicBlock;
 class CallBase;
 class Function;
 class Module;
 class OptimizationRemarkEmitter;
 
 /// There are 3 scenarios we can use the InlineAdvisor:
 /// - Default - use manual heuristics.
 ///
+/// - MandatoryOnly - only mandatory inlinings (i.e. AlwaysInline).
+///
 /// - Release mode, the expected mode for production, day to day deployments.
 /// In this mode, when building the compiler, we also compile a pre-trained ML
 /// model to native code, and link it as a static library. This mode has low
 /// overhead and no additional dependencies for the compiler runtime.
 ///
 /// - Development mode, for training new models.
 /// In this mode, we trade off runtime performance for flexibility. This mode
 /// requires the full C Tensorflow API library, and evaluates models
 /// dynamically. This mode also permits generating training logs, for offline
 /// training.
-enum class InliningAdvisorMode : int { Default, Release, Development };
+enum class InliningAdvisorMode : int {
+  Default,
+  MandatoryOnly,
+  Release,
+  Development
+};
 
 class InlineAdvisor;
 /// Capture state between an inlining decision having had been made, and
 /// its impact being observable. When collecting model training data, this
 /// allows recording features/decisions/partial reward data sets.
 ///
 /// Derivations of this type are expected to be tightly coupled with their
 /// InliningAdvisors. The base type implements the minimal contractual

 private:
   std::unique_ptr<InlineAdvice> getAdvice(CallBase &CB) override;
 
   void onPassExit() override { freeDeletedFunctions(); }
 
   InlineParams Params;
 };
 
+/// Advisor recommending only mandatory (AlwaysInline) cases.
+class MandatoryInlineAdvisor final : public InlineAdvisor {
+  std::unique_ptr<InlineAdvice> getAdvice(CallBase &CB) override;
+
+public:
+  MandatoryInlineAdvisor(FunctionAnalysisManager &FAM) : InlineAdvisor(FAM) {}
+
+  enum class MandatoryInliningKind { NotMandatory, Always, Never };
+
+  static MandatoryInliningKind getMandatoryKind(CallBase &CB,
+                                                FunctionAnalysisManager &FAM,
+                                                OptimizationRemarkEmitter &ORE);
+};
+
 /// The InlineAdvisorAnalysis is a module pass because the InlineAdvisor
 /// needs to capture state right before inlining commences over a module.
 class InlineAdvisorAnalysis : public AnalysisInfoMixin<InlineAdvisorAnalysis> {
 public:
   static AnalysisKey Key;
   InlineAdvisorAnalysis() = default;
   struct Result {
     Result(Module &M, ModuleAnalysisManager &MAM) : M(M), MAM(MAM) {}

llvm/include/llvm/Passes/PassBuilder.h

public:
   ///
   /// \p Phase indicates the current ThinLTO phase.
   ModulePassManager buildModuleSimplificationPipeline(OptimizationLevel Level,
                                                       ThinLTOPhase Phase);
 
   /// Construct the module pipeline that performs inlining as well as
   /// the inlining-driven cleanups.
   ModuleInlinerWrapperPass buildInlinerPipeline(OptimizationLevel Level,
-                                                ThinLTOPhase Phase);
+                                                ThinLTOPhase Phase,
+                                                bool MandatoryOnly);
 
   /// Construct the core LLVM module optimization pipeline.
   ///
   /// This pipeline focuses on optimizing the execution speed of the IR. It
   /// uses cost modeling and thresholds to balance code growth against runtime
   /// improvements. It includes vectorization and other information destroying
   /// transformations. It also cannot generally be run repeatedly on a module
   /// without potentially seriously regressing either runtime performance of

llvm/lib/Analysis/InlineAdvisor.cpp

 
 bool InlineAdvisorAnalysis::Result::tryCreate(InlineParams Params,
                                               InliningAdvisorMode Mode) {
   auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   switch (Mode) {
   case InliningAdvisorMode::Default:
     Advisor.reset(new DefaultInlineAdvisor(FAM, Params));
     break;
+  case InliningAdvisorMode::MandatoryOnly:
+    Advisor.reset(new MandatoryInlineAdvisor(FAM));
+    break;
   case InliningAdvisorMode::Development:
 #ifdef LLVM_HAVE_TF_API
     Advisor =
         llvm::getDevelopmentModeAdvisor(M, MAM, [&FAM, Params](CallBase &CB) {
           auto OIC = getDefaultInlineAdvice(CB, FAM, Params);
           return OIC.hasValue();
         });
 #endif
ORE.emit([&]() {
     Remark << ore::NV("Caller", &Caller);
     if (ForProfileContext)
       Remark << " to match profiling context";
     Remark << " with " << IC;
     addLocationToRemarks(Remark, DLoc);
     return Remark;
   });
 }
+
+std::unique_ptr<InlineAdvice> MandatoryInlineAdvisor::getAdvice(CallBase &CB) {
+  auto &Caller = *CB.getCaller();
+  auto &Callee = *CB.getCalledFunction();
+  auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(Caller);
+
+  bool Advice = MandatoryInliningKind::Always ==
+                    MandatoryInlineAdvisor::getMandatoryKind(CB, FAM, ORE) &&
+                &Caller != &Callee;
+  return std::make_unique<InlineAdvice>(this, CB, ORE, Advice);
+}
+
+MandatoryInlineAdvisor::MandatoryInliningKind
+MandatoryInlineAdvisor::getMandatoryKind(CallBase &CB,
+                                         FunctionAnalysisManager &FAM,
+                                         OptimizationRemarkEmitter &ORE) {
+  auto &Callee = *CB.getCalledFunction();
+
+  auto GetTLI = [&](Function &F) -> const TargetLibraryInfo & {
+    return FAM.getResult<TargetLibraryAnalysis>(F);
+  };
+
+  auto &TIR = FAM.getResult<TargetIRAnalysis>(Callee);
+
+  auto TrivialDecision =
+      llvm::getAttributeBasedInliningDecision(CB, &Callee, TIR, GetTLI);
+
+  if (TrivialDecision.hasValue()) {
+    if (TrivialDecision->isSuccess())
+      return MandatoryInliningKind::Always;
+    else
+      return MandatoryInliningKind::Never;
+  }
+  return MandatoryInliningKind::NotMandatory;
+}

llvm/lib/Analysis/MLInlineAdvisor.cpp

 
 std::unique_ptr<InlineAdvice> MLInlineAdvisor::getAdvice(CallBase &CB) {
   auto &Caller = *CB.getCaller();
   auto &Callee = *CB.getCalledFunction();
 
   auto GetAssumptionCache = [&](Function &F) -> AssumptionCache & {
     return FAM.getResult<AssumptionAnalysis>(F);
   };
-  auto GetTLI = [&](Function &F) -> const TargetLibraryInfo & {
-    return FAM.getResult<TargetLibraryAnalysis>(F);
-  };
-
   auto &TIR = FAM.getResult<TargetIRAnalysis>(Callee);
   auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(Caller);
 
-  auto TrivialDecision =
-      llvm::getAttributeBasedInliningDecision(CB, &Callee, TIR, GetTLI);
-
+  auto MandatoryKind = MandatoryInlineAdvisor::getMandatoryKind(CB, FAM, ORE);
   // If this is a "never inline" case, there won't be any changes to internal
   // state we need to track, so we can just return the base InlineAdvice, which
   // will do nothing interesting.
   // Same thing if this is a recursive case.
-  if ((TrivialDecision.hasValue() && !TrivialDecision->isSuccess()) ||
+  if (MandatoryKind == MandatoryInlineAdvisor::MandatoryInliningKind::Never ||
       &Caller == &Callee)
     return std::make_unique<InlineAdvice>(this, CB, ORE, false);
 
-  bool Mandatory = TrivialDecision.hasValue() && TrivialDecision->isSuccess();
+  bool Mandatory =
+      MandatoryKind == MandatoryInlineAdvisor::MandatoryInliningKind::Always;
 
   // If we need to stop, we won't want to track anymore any state changes, so
   // we just return the base InlineAdvice, which acts as a noop.
   if (ForceStop) {
     ORE.emit([&] {
       return OptimizationRemarkMissed(DEBUG_TYPE, "ForceStop", &CB)
              << "Won't attempt inlining because module size grew too much.";
     });

llvm/lib/Passes/PassBuilder.cpp

static cl::opt<bool>
     EnablePGOInlineDeferral("enable-npm-pgo-inline-deferral", cl::init(true),
                             cl::Hidden,
                             cl::desc("Enable inline deferral during PGO"));
 
 static cl::opt<bool> EnableMemProfiler("enable-mem-prof", cl::init(false),
                                        cl::Hidden, cl::ZeroOrMore,
                                        cl::desc("Enable memory profiler"));
 
+static cl::opt<bool> PerformMandatoryInliningsFirst(
+    "mandatory-inlining-first", cl::init(true), cl::Hidden, cl::ZeroOrMore,
+    cl::desc("Perform mandatory inlinings module-wide, before performing "
+             "inlining."));
+
 PipelineTuningOptions::PipelineTuningOptions() {
   LoopInterleaving = true;
   LoopVectorization = true;
   SLPVectorization = false;
   LoopUnrolling = true;
   ForgetAllSCEVInLoopUnroll = ForgetSCEVInLoopUnroll;
   Coroutines = false;
   LicmMssaOptCap = SetLicmMssaOptCap;

 }
 
 static InlineParams
 getInlineParamsFromOptLevel(PassBuilder::OptimizationLevel Level) {
   return getInlineParams(Level.getSpeedupLevel(), Level.getSizeLevel());
 }
 
 ModuleInlinerWrapperPass
-PassBuilder::buildInlinerPipeline(OptimizationLevel Level, ThinLTOPhase Phase) {
+PassBuilder::buildInlinerPipeline(OptimizationLevel Level, ThinLTOPhase Phase,
+                                  bool MandatoryOnly) {
   InlineParams IP = getInlineParamsFromOptLevel(Level);
   if (Phase == PassBuilder::ThinLTOPhase::PreLink && PGOOpt &&
       PGOOpt->Action == PGOOptions::SampleUse)
     IP.HotCallSiteThreshold = 0;
 
   if (PGOOpt)
     IP.EnableDeferral = EnablePGOInlineDeferral;
 
-  ModuleInlinerWrapperPass MIWP(IP, DebugLogging, UseInlineAdvisor,
-                                MaxDevirtIterations);
+  ModuleInlinerWrapperPass MIWP(
+      IP, DebugLogging,
+      (MandatoryOnly ? InliningAdvisorMode::MandatoryOnly : UseInlineAdvisor),
 
+  if (MandatoryOnly)
+    return MIWP;
+
   // Require the GlobalsAA analysis for the module so we can query it within
   // the CGSCC pipeline.
   MIWP.addRequiredModuleAnalysis<GlobalsAA>();
 
   // Require the ProfileSummaryAnalysis for the module so we can query it within
   // the inliner pass.
   MIWP.addRequiredModuleAnalysis<ProfileSummaryAnalysis>();
 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   if (PGOOpt && Phase != ThinLTOPhase::PostLink &&
       PGOOpt->CSAction == PGOOptions::CSIRInstr)
     MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->CSProfileGenFile));
 
   // Synthesize function entry counts for non-PGO compilation.
   if (EnableSyntheticCounts && !PGOOpt)
     MPM.addPass(SyntheticCountsPropagation());
 
-  MPM.addPass(buildInlinerPipeline(Level, Phase));
+  if (PerformMandatoryInliningsFirst)
+    MPM.addPass(buildInlinerPipeline(Level, Phase, /*MandatoryOnly=*/true));
+  MPM.addPass(buildInlinerPipeline(Level, Phase, /*MandatoryOnly=*/false));
 
   if (EnableMemProfiler && Phase != ThinLTOPhase::PreLink) {
     MPM.addPass(createModuleToFunctionPassAdaptor(MemProfilerPass()));
     MPM.addPass(ModuleMemProfilerPass());
   }
 
   return MPM;
 }

llvm/lib/Passes/PassRegistry.def

 MODULE_PASS("globaldce", GlobalDCEPass())
 MODULE_PASS("globalopt", GlobalOptPass())
 MODULE_PASS("globalsplit", GlobalSplitPass())
 MODULE_PASS("hotcoldsplit", HotColdSplittingPass())
 MODULE_PASS("hwasan", HWAddressSanitizerPass(false, false))
 MODULE_PASS("khwasan", HWAddressSanitizerPass(true, true))
 MODULE_PASS("inferattrs", InferFunctionAttrsPass())
 MODULE_PASS("inliner-wrapper", ModuleInlinerWrapperPass())
+MODULE_PASS("always-inliner-wrapper", ModuleInlinerWrapperPass(
+  getInlineParams(), 
+  DebugLogging, 
+  InliningAdvisorMode::MandatoryOnly))
 MODULE_PASS("insert-gcov-profiling", GCOVProfilerPass())
 MODULE_PASS("instrorderfile", InstrOrderFilePass())
 MODULE_PASS("instrprof", InstrProfiling())
 MODULE_PASS("internalize", InternalizePass())
 MODULE_PASS("invalidate<all>", InvalidateAllAnalysesPass())
 MODULE_PASS("ipsccp", IPSCCPPass())
 MODULE_PASS("print-ir-similarity", IRSimilarityAnalysisPrinterPass(dbgs()))
 MODULE_PASS("loop-extract", LoopExtractorPass())

 MODULE_PASS("print-must-be-executed-contexts", MustBeExecutedContextPrinterPass(dbgs()))
 MODULE_PASS("print-stack-safety", StackSafetyGlobalPrinterPass(dbgs()))
 MODULE_PASS("print<module-debuginfo>", ModuleDebugInfoPrinterPass(dbgs()))
 MODULE_PASS("rewrite-statepoints-for-gc", RewriteStatepointsForGC())
 MODULE_PASS("rewrite-symbols", RewriteSymbolPass())
 MODULE_PASS("rpo-function-attrs", ReversePostOrderFunctionAttrsPass())
 MODULE_PASS("sample-profile", SampleProfileLoaderPass())
 MODULE_PASS("scc-oz-module-inliner",
-  buildInlinerPipeline(OptimizationLevel::Oz, ThinLTOPhase::None))
+  buildInlinerPipeline(OptimizationLevel::Oz, ThinLTOPhase::None, 
+  /*MandatoryOnly=*/false))
 MODULE_PASS("loop-extract-single", LoopExtractorPass(1))
 MODULE_PASS("oz-module-optimizer",
   buildModuleOptimizationPipeline(OptimizationLevel::Oz, /*LTOPreLink*/false))
 MODULE_PASS("strip", StripSymbolsPass())
 MODULE_PASS("strip-dead-debug-info", StripDeadDebugInfoPass())
 MODULE_PASS("strip-dead-prototypes", StripDeadPrototypesPass())
 MODULE_PASS("strip-debug-declare", StripDebugDeclarePass())
 MODULE_PASS("strip-nondebug", StripNonDebugSymbolsPass())

llvm/lib/Transforms/IPO/Inliner.cpp

 /// prior to LLVM's code generator having support for stack coloring based on
 /// lifetime markers. It is now in the process of being removed. To experiment
 /// with disabling it and relying fully on lifetime marker based stack
 /// coloring, you can pass this flag to LLVM.
 static cl::opt<bool>
     DisableInlinedAllocaMerging("disable-inlined-alloca-merging",
                                 cl::init(false), cl::Hidden);
 
-/// Flag to disable adding AlwaysInlinerPass to ModuleInlinerWrapperPass.
-/// TODO: remove this once this has is baked in for long enough.
-static cl::opt<bool> DisableAlwaysInlinerInModuleWrapper(
-    "disable-always-inliner-in-module-wrapper", cl::init(false), cl::Hidden);
-
 namespace {
 
 enum class InlinerFunctionImportStatsOpts {
   No = 0,
   Basic = 1,
   Verbose = 2,
 };
 
PreservedAnalyses ModuleInlinerWrapperPass::run(Module &M,
   auto &IAA = MAM.getResult<InlineAdvisorAnalysis>(M);
   if (!IAA.tryCreate(Params, Mode)) {
     M.getContext().emitError(
         "Could not setup Inlining Advisor for the requested "
         "mode and/or options");
     return PreservedAnalyses::all();
   }
 
-  if (!DisableAlwaysInlinerInModuleWrapper)
-    MPM.addPass(AlwaysInlinerPass());
   // We wrap the CGSCC pipeline in a devirtualization repeater. This will try
   // to detect when we devirtualize indirect calls and iterate the SCC passes
   // in that case to try and catch knock-on inlining or function attrs
   // opportunities. Then we add it to the module pipeline by walking the SCCs
   // in postorder (or bottom-up).
   // If MaxDevirtIterations is 0, we just don't use the devirtualization
   // wrapper.
   if (MaxDevirtIterations == 0)

llvm/test/Transforms/Inline/ML/bounds-checks-rewards.ll

 ; Test behavior when inlining policy grows size out of control.
 ; In all cases, the end result is the same: mandatory inlinings must happen.
 ; Also in all cases, we don't record the mandatory inlining (there's nothing to
 ; learn from it).
 ; However, when we discover we 'trip' over the artificially-low size increase 
 ; factor, we penalize the 'bad' decision.
 ; REQUIRES: have_tf_api
 ;
 ; When the bounds are very wide ("no bounds"), all inlinings happen.
-; RUN: opt -passes=scc-oz-module-inliner -ml-inliner-ir2native-model=%S/../../../../unittests/Analysis/Inputs/ir2native_x86_64_model -ml-inliner-model-under-training=%S/../../../../lib/Analysis/models/inliner -training-log=- -enable-ml-inliner=development -ml-advisor-size-increase-threshold=10.0 -disable-always-inliner-in-module-wrapper -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=NOBOUNDS
+; RUN: opt -passes=scc-oz-module-inliner -ml-inliner-ir2native-model=%S/../../../../unittests/Analysis/Inputs/ir2native_x86_64_model -ml-inliner-model-under-training=%S/../../../../lib/Analysis/models/inliner -training-log=- -enable-ml-inliner=development -ml-advisor-size-increase-threshold=10.0 -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=NOBOUNDS
 ;
 ; When the bounds are very restrictive, the first inlining happens but it's
 ; considered as "bad" (since it trips over the bounds) and its reward is a
 ; penalty. However, the mandatory inlining, which is considered next, happens.
 ; No other inlinings happend.
-; RUN: opt -passes=scc-oz-module-inliner -ml-inliner-ir2native-model=%S/../../../../unittests/Analysis/Inputs/ir2native_x86_64_model -ml-inliner-model-under-training=%S/../../../../lib/Analysis/models/inliner -training-log=- -enable-ml-inliner=development -ml-advisor-size-increase-threshold=1.0 -disable-always-inliner-in-module-wrapper -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOUNDS
+; RUN: opt -passes=scc-oz-module-inliner -ml-inliner-ir2native-model=%S/../../../../unittests/Analysis/Inputs/ir2native_x86_64_model -ml-inliner-model-under-training=%S/../../../../lib/Analysis/models/inliner -training-log=- -enable-ml-inliner=development -ml-advisor-size-increase-threshold=1.0 -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOUNDS
 ;
 ; With more restrictive bounds, the first inlining happens and is OK. The
 ; mandatory inlining happens next, and it trips over the bounds, which then
 ; forces no further inlinings.
-; RUN: opt -passes=scc-oz-module-inliner -ml-inliner-ir2native-model=%S/../../../../unittests/Analysis/Inputs/ir2native_x86_64_model -ml-inliner-model-under-training=%S/../../../../lib/Analysis/models/inliner -training-log=- -enable-ml-inliner=development -ml-advisor-size-increase-threshold=1.1 -disable-always-inliner-in-module-wrapper -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=RELAXED-BOUNDS
+; RUN: opt -passes=scc-oz-module-inliner -ml-inliner-ir2native-model=%S/../../../../unittests/Analysis/Inputs/ir2native_x86_64_model -ml-inliner-model-under-training=%S/../../../../lib/Analysis/models/inliner -training-log=- -enable-ml-inliner=development -ml-advisor-size-increase-threshold=1.1 -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=RELAXED-BOUNDS
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 declare i64 @f1()
 
 define i64 @may_not_be_inlined() {
   %r = call i64 @f1()

 ; CHECK-LABEL: @top
 ; must_be_inlined must always be inlined, so we won't find a call to it in @top()
 ; CHECK-NOT: call i64 @must_be_inlined
 ; @some-function isn't mandatory, and when we set the increase threshold too low,
 ; it won't be inlined.
 ; NOBOUNDS-NOT: @may_not_be_inlined
 ; RELAXED-BOUNDS: call i64 @may_not_be_inlined
 ; BOUNDS: call i64 @may_not_be_inlined
 No newline at end of file

llvm/test/Transforms/Inline/ML/bounds-checks.ll

 ; Test behavior when inlining policy grows size out of control.
 ; In all cases, the end result is the same: mandatory inlinings must happen.
 ; However, when we discover we 'trip' over the artificially-low size increase 
 ; factor, we don't inline anymore.
 ; REQUIRES: have_tf_aot
 ; RUN: opt -passes=scc-oz-module-inliner -enable-ml-inliner=release -ml-advisor-size-increase-threshold=10.0 -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=NOBOUNDS
-; RUN: opt -passes=scc-oz-module-inliner -enable-ml-inliner=release -ml-advisor-size-increase-threshold=1.0 -disable-always-inliner-in-module-wrapper -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOUNDS
+; RUN: opt -passes=scc-oz-module-inliner -enable-ml-inliner=release -ml-advisor-size-increase-threshold=1.0 -S < %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOUNDS
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 declare i64 @f1()
 
 define i64 @f2() #0 {
   %r = call i64 @f1()

 
 ; CHECK-LABEL: @top
 ; f2 must always be inlined, so we won't find a call to it in @top()
 ; CHECK-NOT: call i64 @f2
 ; @some-function isn't mandatory, and when we set the increase threshold too low,
 ; it won't be inlined.
 ; NOBOUNDS-NOT: @some_function
 ; BOUNDS: call i64 @some_function
 No newline at end of file

llvm/test/Transforms/Inline/inline_stats.ll

 ; First with legacy PM
 ; RUN: opt -S -inline -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s -check-prefix=CHECK-BASIC -check-prefix=CHECK
 ; RUN: opt -S -inline -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s -check-prefix="CHECK-VERBOSE" -check-prefix=CHECK
 
 ; Do again with new PM
 ; RUN: opt -S -passes=inline -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s -check-prefix=CHECK-BASIC -check-prefix=CHECK
 ; RUN: opt -S -passes=inline -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s -check-prefix="CHECK-VERBOSE" -check-prefix=CHECK
 
-; RUN: opt -S -passes=inliner-wrapper -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s -check-prefix=WRAPPER-BASIC -check-prefix=WRAPPER
-; RUN: opt -S -passes=inliner-wrapper -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s -check-prefix=WRAPPER-VERBOSE -check-prefix=WRAPPER
+; RUN: opt -S -passes=inliner-wrapper -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s -check-prefix=CHECK-BASIC -check-prefix=CHECK
+; RUN: opt -S -passes=inliner-wrapper -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s -check-prefix="CHECK-VERBOSE" -check-prefix=CHECK
+
+; RUN: opt -S -passes=always-inliner-wrapper,inliner-wrapper -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s -check-prefix=WRAPPER-BASIC -check-prefix=WRAPPER
+; RUN: opt -S -passes=always-inliner-wrapper,inliner-wrapper -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s -check-prefix=WRAPPER-VERBOSE -check-prefix=WRAPPER
 
 ; CHECK: ------- Dumping inliner stats for [<stdin>] -------
 ; CHECK-BASIC-NOT: -- List of inlined functions:
 ; CHECK-BASIC-NOT: -- Inlined not imported function
 ; CHECK-VERBOSE: -- List of inlined functions:
 ; CHECK-VERBOSE: Inlined not imported function [internal2]: #inlines = 6, #inlines_to_importing_module = 2
 ; CHECK-VERBOSE: Inlined imported function [external2]: #inlines = 4, #inlines_to_importing_module = 1
 ; CHECK-VERBOSE: Inlined imported function [external1]: #inlines = 3, #inlines_to_importing_module = 2