[PATCH] D22491: Added ThinLTO inlining statistics

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 12:41:20 PDT 2016


Prazek added inline comments.

================
Comment at: lib/Transforms/IPO/Inliner.cpp:597
@@ +596,3 @@
+    ImportedFunctionsStats.dump(InlinerFunctionImportStats ==
+                                    InlinerFunctionImportStatsOpts::Verbose,
+                                CG.getModule());
----------------
tejohnson wrote:
> This indentation doesn't look right to me - make sure you clang format.
This was actually formatted with clang-format. I guess the arguments are not in the same column because it would look the same as the next arguments to function.

================
Comment at: test/Transforms/Inline/inline_stats.ll:17
@@ +16,3 @@
+; CHECK: Inlined not imported functions: 1 [33.33%]
+; CHECK: non-imported functions inlined into importing module: 1 [100%]
+
----------------
tejohnson wrote:
> tejohnson wrote:
> > It isn't clear what the percentages represent (e.g. some are percent of total # functions and some are percent of inlined functions). Probably need to add a tag - e.g.:
> > 
> > ; CHECK: inlined functions: 5 [50% of all functions]
> > ; CHECK: imported functions inlined: 4 [57.14% of imported functions]
> > ; CHECK: imported functions inlined into importing module: 3 [75% of inlined imported functions]
> > ; CHECK: Inlined not imported functions: 1 [33.33% of non-imported functions]
> > ; CHECK: non-imported functions inlined into importing module: 1 [100% of non-imported inlined functions]
> > 
> > However, after typing that out, I think for the stat relating to how many are inlined into the importing module, it would be much more useful to have the percentage in terms of the total number of that type of function. E.g. instead of expressing the "imported functions inlined into importing module" as a percentage of "imported functions inlined", it would be more useful to have it be a percentage of "imported functions". Ditto for non-imported functions inlined into importing module.
> Could you add one more stat - the # of imported functions not inlined into importing module (along with percentage of all imported functions)? I know it is computable from the other stats, but it would be useful to be able to get this directly.
I am not sure if it is good way, but I will change it. What I am woried is that the last stat won't tell much at the fist glance, because the number will be low

================
Comment at: test/Transforms/Inline/inline_stats.ll:18
@@ +17,3 @@
+; CHECK: non-imported functions inlined into importing module: 1 [33.33% of non-imported functions]
+
+define void @internal() {
----------------
you mean substract imported functions from inlined imported functions?

like
 imported functions not inlined anywhere: 3 [~43% of imported functions]


https://reviews.llvm.org/D22491





More information about the llvm-commits mailing list