[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