[PATCH] D138602: [WIP] Alwaysinliner time explosion with new pass manager

[Davide Italiano via Phabricator via llvm-commits]

davide created this revision.
davide added reviewers: davidxl, aeubanks, t.p.northover, friss, ab.
Herald added subscribers: ormris, hiraditya.
Herald added a project: All.
davide requested review of this revision.
Herald added a project: LLVM.

There have been problems with the inliner taking a long time in the new pass manager before, discussed in https://reviews.llvm.org/D98481, https://reviews.llvm.org/D120584, and the finally landed https://reviews.llvm.org/D121084.

Unfortunately the fix that landed is a cost-model tweak to suppress exponential inlining so doesn't apply to the alwaysinline case. I've had some success with going the D98481 <https://reviews.llvm.org/D98481> route internally, but that has the significant(!) disadvantage of actually disabling inlining of some alwaysinline functions (no-one seems to have noticed though).

Just before Clang removed support for the legacy pass-manager entirely, the soon-to-be-attached reduced case took ~9s with the new one, and 0.04s with the old one. ToT is (as expected) also 9s.

I understand this might not be a definitive solution, but I'd like to use this review to brainstorm a path forward.

see: https://github.com/llvm/llvm-project/issues/59126


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138602

Files:
  llvm/lib/Transforms/IPO/Inliner.cpp


Index: llvm/lib/Transforms/IPO/Inliner.cpp
===================================================================
--- llvm/lib/Transforms/IPO/Inliner.cpp
+++ llvm/lib/Transforms/IPO/Inliner.cpp
@@ -890,6 +890,30 @@
         continue;
       }
 
+      // Do not inline a function involved in a mutual-recursive cycle. Due to
+      // the lack of a global inline history, such inlining may cause an
+      // exponential size growth as the inlining extends up along the call
+      // graph. Consider the following example,
+      //   void A() { B(); B();}
+      //   void B() { A(); }
+      //   void C() { B(); }
+      //   void D() { C(); }
+      // When processing C, inlining B will result in
+      //   void C() { B(); B(); }
+      // When processing D, inlining C and futher the two B callsites will
+      // result in
+      //   void D() { B(); B(); B(); B(); }
+      // Note that we are only checking mutual-recursion here. Self-recurision
+      // is handled by inline cost analyzer.
+      if (CalleeSCC != C && CalleeSCC->size() > 1 &&
+          Callee.hasFnAttribute(Attribute::AlwaysInline)) {
+        LLVM_DEBUG(dbgs() << "Skipping inlining a node that is involved in a "
+                             "mutual-recursive SCC"
+                          << F.getName() << " -> " << Callee.getName() << "\n");
+        setInlineRemark(*CB, "recursive");
+        continue;
+      }
+
       std::unique_ptr<InlineAdvice> Advice =
           Advisor.getAdvice(*CB, OnlyMandatory);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138602.477586.patch
Type: text/x-patch
Size: 1504 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221123/efe2b6a8/attachment.bin>