[PATCH] D22491: Added ThinLTO inlining statistics

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 16:47:31 PDT 2016


Prazek marked 4 inline comments as done.

================
Comment at: lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp:106
@@ +105,3 @@
+    if (Node.second.Imported) {
+      InlinedImportedFunctionsCount += (Node.second.NumberOfInlines > 0) * 1;
+      InlinedImportedFunctionsToImportingModuleCount +=
----------------
eraman wrote:
> Node.second.NumberOfInlines is > 0 here, so you don't need the (Node.second.NumberOfInlines > 0) * 1. You could simply increment the count by 1.
I know about this, I wanted to make it more explicit. The implicit conversion from bool is bugprone, so I will just add int() here so
int(Node.second.NumberOfInlines > 0)

================
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() {
----------------
tejohnson wrote:
> tejohnson wrote:
> > Prazek wrote:
> > > you mean substract imported functions from inlined imported functions?
> > > 
> > > like
> > >  imported functions not inlined anywhere: 3 [~43% of imported functions]
> > Meaning subtract "imported functions inlined into importing module" from "imported functions" (4 in the above example).
> I'm less sure about how we will use the last one (non-imported functions inlined into importing module vs non-imported functions inlined anywhere), but for the imported functions what we primarily want to see is how many/what percentage were inlined into the importing module.
Is this line ok?

imported functions inlined into importing module: 3 [42.86% of imported functions], left: 4 [57.14% of imported functions]


https://reviews.llvm.org/D22491





More information about the llvm-commits mailing list