[PATCH] D136383: [PartialInlining] Enable recursive partial inlining.

[Mark Lacey via Phabricator via llvm-commits]

rudkx added a comment.

In D136383#3924032 <https://reviews.llvm.org/D136383#3924032>, @efriedma wrote:

> The obvious concern for making inlining more aggressive is potential codesize growth.  Does the cost model prevent us from cloning the function multiple times?  How much growth are we talking about for practical code?

Hi @efriedma. The majority of the body of the function will get cloned exactly once. The early-exit portion of the function (leading to the first branch) will get cloned once per call-site. Since the only change here is to also allow partial inlining to happen for recursive functions, I would expect code growth in general to be quite small over just having this enabled for non-recursive functions. In other words, I would expect if there were a code growth problem as a result of partial inlining we'd already be seeing it since non-recursive functions are going to be the much more common case.

Having said that, I was hoping to try something like a bootstrap build of clang to see if there was any noticeable impact, but the instructions I found online for doing those builds don't seem to work. In an internal test suite I found that this only fired a handful of times (most likely because the code shape that this pass detects is very specific, and the cost model is pretty conservative).

If you still have concerns I'd be happy to gather some data if you have a suggestion of what you think would be meaningful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136383