[PATCH] D70584: [AutoFDO] Statistic for context sensitive profile guided inlining

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

> However the reality is most of the inlining actually happens during regular inliner.

I would imagine among all the functions with inline instance in profile, only those small and warm/cold functions which are not inlined in early inliner will be inlined in regular inliner. The number of those small/warm and small/cold functions may be large. We found it was helpful to inline warm functions before (but have to pay some cost of code size increase. It is better to only inline small/warm functions). For small and cold functions, I think it doesn't matter whether they are inlined early or late.

It is helpful to collect some optimization remarks here, so thanks for the patch.



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:879
   if (Cost.isNever()) {
-    ORE->emit(OptimizationRemark(DEBUG_TYPE, "Not inline", DLoc, BB)
+    ORE->emit(OptimizationRemark("sample-profile-inline", "NotInline", DLoc, BB)
               << "incompatible inlining");
----------------
The first parameter in the declaration of OptimizationRemark is "const char *PassName", so why not use DEBUG_TYPE?  

I feel "NeverInline" may be more clear than "NotInline" in terms of showing it is illegal to inline. 


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:941-952
+      else {
+        for (auto I : Candidates) {
+          Function *CalledFunction = CallSite(I).getCalledFunction();
+          if (CalledFunction) {
+            ORE->emit(OptimizationRemark("sample-profile-inline", "NotInline",
+                                         I->getDebugLoc(), I->getParent())
+                      << "previous inline not repeated '"
----------------
Not inlined candidate may be reported multiple times here because of the iterative outer loop. 

I guess you put the OptimizationRemark here because you want to know the exact reason of why the candidate with inline instance in profile is not inlined (here the reason is not hot enough), then some more information should be emitted to explain it.

If you don't care the exact reason, then it is better to generate the optimization remark in the loop iterating localNotInlinedCallSites.  localNotInlinedCallSites contains all the candidates with inline instance in profile but not being inlined for whatever reason including the reason of "not hot enough". 


================
Comment at: llvm/test/Transforms/SampleProfile/inline-stats.ll:2
+; REQUIRES: asserts
+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.prof -stats -S 2>&1 | FileCheck %s
+
----------------
Test new pass manager as well.


================
Comment at: llvm/test/Transforms/SampleProfile/inline-stats.ll:29
+  store i32 %y, i32* %y.addr, align 4
+  %0 = load i32, i32* %x.addr, align 4, !dbg !11
+  %1 = load i32, i32* %y.addr, align 4, !dbg !11
----------------
rename the vars with just a number.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70584