[PATCH] D22491: Added ThinLTO inlining statistics

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 19 14:49:59 PDT 2016


Prazek added inline comments.

================
Comment at: include/llvm/Transforms/IPO/InlinerStats.h:22
@@ +21,3 @@
+/// Then starting from non external functions that have some inlined calls
+/// inside, it walks to every inlined function and increment counter.
+///
----------------
mehdi_amini wrote:
> Prazek wrote:
> > tejohnson wrote:
> > > mehdi_amini wrote:
> > > > It is not clear to me why you need a graph and can't just increment counters as the inlined moves on?
> > > This is to distinguish between inlines that eventually make it into a non-imported function (i.e. what Piotr refers to as a "real inline" and I am suggesting something like "inline to importing module"). I.e. if we have call chain A->B->C where A was originally in the module (non-imported) and B/C were imported, when we inline C into B we don't know if they will ultimately be inlined into A. Presumably there could be another SCC that we perform inlines on in between the B->C and A->BC inlines so we can't easily track this without some side structure.
> > indeed. I am gonna change that comment to be more clear
> OK for a side structure, does it need to build a graph? I'm not sure if a map `F -> { real, unreal }` + the global counters would not suffice?
> 
> I mean, it depends on the semantics of these counters, and I'm still not sure about this. For example:
> 
> `A->B->C` and `D->B->C`, with C and B the only imported functions. If we import entirely (C into B, and B into A and D), what is expected to be computed?
> 
Then the number of inlines for B: 2, for C: 1, and number of inline to importing module (aka real inlines) will be B: 2, C: 2.

The graph is to calculate the number of real inlines. Without it we won't know if the function was realy inlined into some function that wasn't imported and we won't be able to see for example
that all functions that we have imported were never really inlined.


https://reviews.llvm.org/D22491





More information about the llvm-commits mailing list