[PATCH] D145516: [Inliner] Avoid excessive inlining through devirtualised calls

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

actually I think the inline history feature is relevant here. normally we'll detect this sort of issue with the inline history and bail out when we try to inline `rec_call_to_longname` again from `longname`, and in fact `-debug` shows a lot of `Skipping inlining due to history`. but that's per-inline invocation, and due to the multiple callers, that gets forgotten every time we go up to the next caller. I think marking the call with `noinline` should allow us to remember this across inline invocations.

  diff --git a/llvm/lib/Transforms/IPO/Inliner.cpp b/llvm/lib/Transforms/IPO/Inliner.cpp
  index 01808b3d14fe..a50147f07f68 100644
  --- a/llvm/lib/Transforms/IPO/Inliner.cpp
  +++ b/llvm/lib/Transforms/IPO/Inliner.cpp
  @@ -333,6 +333,7 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
           LLVM_DEBUG(dbgs() << "Skipping inlining due to history: " << F.getName()
                             << " -> " << Callee.getName() << "\n");
           setInlineRemark(*CB, "recursive");
  +        CB->setIsNoInline();
           continue;
         }

this is fairly similar to "inlining through a child SCC" problem that was mitigated with `function-inline-cost-multiplier`. I think that this noinline patch will take care of many cases that `function-inline-cost-multiplier` initially set out fix (and the first iteration of that patch was to add `noinline` instead of a cost multiplier), but `function-inline-cost-multiplier` isn't completely redundant because this `noinline` will only kick in if we see that we've revisited a callee, but `function-inline-cost-multiplier` will still work with inlining through larger SCCs if we don't revisit the first callee quickly


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

https://reviews.llvm.org/D145516