mtrofin added a comment.

In D91567#2398440 <https://reviews.llvm.org/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 
<https://reviews.llvm.org/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 <https://reviews.llvm.org/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?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91567

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D91567... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... David Blaikie via Phabricator via cfe-commits
    • [PATCH] D... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... David Blaikie via Phabricator via cfe-commits
    • [PATCH] D... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... Arthur Eubanks via Phabricator via cfe-commits
    • [PATCH] D... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... David Blaikie via Phabricator via cfe-commits
    • [PATCH] D... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... David Blaikie via Phabricator via cfe-commits
    • [PATCH] D... Arthur Eubanks via Phabricator via cfe-commits
    • [PATCH] D... Mircea Trofin via Phabricator via cfe-commits
    • [PATCH] D... Google Contributors to LLVM via Phabricator via cfe-commits

Reply via email to