[PATCH] D120584: [NewPM][Inliner] Mark inlined calls to functions in same SCC as callee as noinline

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:942-950
+              // Continually inlining through an SCC can result in huge compile
+              // times and bloated code since we arbitrarily stop at some point
+              // when the inliner decides it's not profitable to inline
+              // anymore. We put a stop at the first potential attempt at
+              // inlining through an SCC by marking the call site as noinline.
+              // This doesn't apply to calls in the same SCC since if we do
+              // inline through the SCC the function will end up being
----------------
Just a suggestion, based on my understanding...


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:952
+              if (CalleeSCC != C &&
+                  CalleeSCC == CG.lookupSCC(CG.get(*NewCallee))) {
+                ICB->addFnAttr(Attribute::NoInline);
----------------
davidxl wrote:
> This feels like very strict. Is it possible to allow some level (under an option) of iterative inlining into the same SCC?     This is because not all cases of inlining into a non-trivial SCC will result in compile time issue due to existing threshold based stopping mechanism (i.e. the new callee in SCC is too large).
> 
> 
This does not restrict inlining into the same SCC. What it prevents is continuously peeling off more and more calls from a *different* non-trivial SCC.

>From what I can tell, this is a rare case, only two programs in llvm-test-suite show any codegen changes with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120584