[PATCH] D22491: Added ThinLTO inlining statistics

[Piotr Padlewski via llvm-commits]

Prazek marked 4 inline comments as done and an inline comment as not done.

================
Comment at: lib/Transforms/IPO/InlinerStats.cpp:51
@@ +50,3 @@
+      dbgs() << "Inlined "
+             << (Node.second.Imported ? "imported " : "not external ")
+             << "function [" << Node.first->getName() << "]"
----------------
tejohnson wrote:
> s/not external/not imported/
> 
> Also, it would be useful to know here whether the caller that we are inlining into is imported or not.
But we can have multiple callers. I think the real inlines gives you the information that you need.

================
Comment at: lib/Transforms/IPO/InlinerStats.cpp:61
@@ +60,3 @@
+         << NumberOfRealUniqueInlinedImportedFunctions
+         << "\nNumber of real not external inlined functions: "
+         << NumberOFRealUniqueInlinedNotExternalFunctions << "\n";
----------------
tejohnson wrote:
> Should be trivial I think to compute the reverse as well: how many imported functions were not inlined into a non-imported function (possibly via other inlines).
I am not sure if I should compute it here. The number of all imported functions is counted in importer.

================
Comment at: lib/Transforms/IPO/InlinerStats.cpp:72
@@ +71,3 @@
+void InlinerStatistics::dfs(InlineGraphNode *const GraphNode) {
+  for (auto *const InlinedFunctionNode : GraphNode->InlinedFunctions) {
+    InlinedFunctionNode->NumberOfRealInlines++;
----------------
eraman wrote:
> I think the graph can have cycle. Consider a mutual recursion A->B->A. Let's say B gets inlined into A. This would normally result in a self-recursion (A->A). This would prevent A from being inlined into B. But, if we are in a situation where B first gets inlined into A and the A->A call is DCEd. Then A could be inlined into B causing a cycle in your InlineGraph.
> 
> 
fixed.


https://reviews.llvm.org/D22491