[PATCH] D39869: [Inliner] Inline through indirect call sites having !callees metadata

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/IPO/Inliner.cpp:563
+      // call site will now be the direct one the indirect one will be visited
+      // again.
+      Calls.push_back({DirectCS, Calls[Index].second});
----------------
eraman wrote:
> Is there any limit on the size of the callees metadata? Otherwise you might potentially end up promoting a large number of calls and inline them
The CalledValuePropagation pass limits the size of the callees set to 4 functions. But it's probably a good idea to have a limit in the inliner as well. I'll add a command line option for limiting the number of callees we inline for a given call site.


================
Comment at: lib/Transforms/Utils/CallPromotionUtils.cpp:650
+    Function *F = PossibleCallees[0];
+    if (CalleesToIgnore.count(F) || !isLegalToPromote(CS, F))
+      return nullptr;
----------------
eraman wrote:
> It looks like isLegalToPromote doesn't check if the callee's body is available. If F->isDeclaration() is true, is there any reason to promote? The inliner is going to not inline the callee and ends up unnecessarily promote and demote the callee. This is even more important in the versioned case since it invalidates the analysis.
Nice catch! I think this check was present in a previous version of this patch, but it must have gotten lost in an update. I'll add it back.


Repository:
  rL LLVM

https://reviews.llvm.org/D39869