[PATCH] D22491: Added ThinLTO inlining statistics

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Transforms/IPO/InlinerStats.cpp:17
@@ +16,3 @@
+  if (!FunNode.Imported)
+    NonExternalFunctions.push_back(Fun);
+
----------------
Prazek wrote:
> tejohnson wrote:
> > s/NonExternal/NonImported/?
> > 
> > Also, isn't it possible that we would have seen Fun before, in which case it might get pushed here multiple times? Can the NonExternalFunctions list be built by a single walk over the module rather than checking for every single inline? Looking at its use, it appears to only be used once during dumping in calculateRealInlines, maybe we can just walk the functions in the module there instead of keeping a list.
> Ok so there are two things
> 1. It is true that the function might be added here multiple times. I can solve it easily by sorting the vector and then by adding +n to the counter in DFS, where n is how many times Fun have been pushed.
> 
> 2. I think walking all the functions would be much slower. I don't expect this list to be as big as all the functions. probably 5-25% in thinlto. So rather than iterating all the functions, and having some boolean to distinguish if I should start DFS from this or not, I can do the option (1) which I think is better.
How about use a set instead of a list then. It avoids the need to sort.

I'm not sure on why you need a count of how many times it was pushed (inlined into)? Since you keep appending the functions inlined into each function here in its InlinedFunctions list, why do you need to invoke dfs() on the caller each time?


https://reviews.llvm.org/D22491