[PATCH] D95988: [CSSPGO] Process functions in a top-down order on a dynamic call graph.

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

Thanks for adding the support for non-CS profile!

> Extending profile-based top-down order support to non-CS profile. Only adding support for SCC. Indirect call edges are not needed since uninlined counts are not returned to indirect call targets with non-CS profiles.

Indirect call edges are still helpful for non-CS profiles. That is because top-down inlining will be helpful for better non-CS profile matching fundamentally (unless annotated profile can be updated repeatedly, but that is not the case for branch probablity for non-CS profile). Using non top-down order, function may be annotated with outline instance's profile before it can be inlined and get more precise profile with context. Because there are no indirect call edges in the static call graph, it will be helpful to add them based on dynamic call graph, to enforce the top-down order inlining more thoroughly.



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2299-2305
+  for (const auto &Sample : Samples.getBodySamples()) {
+    for (const auto &Target : Sample.second.getCallTargets()) {
+      Function *Callee = SymbolMap.lookup(Target.first());
+      if (Callee && !Callee->isDeclaration())
+        CG[Caller]->addCalledFunction(nullptr, CG[Callee]);
+    }
+  }
----------------
We may not need this block. Top down inlining for non-CS profile is to get more precise profile matching, not to enable more inlining. If there is not inline instance profile for a callsite, early inlining in sample loader won't inline it so it doesn't need to be added into the dynamic call graph. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95988