[PATCH] D17082: FunctionImport: add a progressive heuristic to limit importing too deep in the callgraph

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 11:01:41 PST 2016


tejohnson added a comment.

This is a reasonable heuristic until we have more sophisticated cost/benefit analysis, especially with PGO data. However, there is an issue due to the depth first traversal order, see below.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:268
@@ -256,1 +267,3 @@
+    // Adjust the threshold
+    Threshold = Threshold * ImportInstrFactor;
     F->materialize();
----------------
I think we can get missed importing opportunities due to the depth-first nature of the current traversal. We push_back newly identified external calls (which have the reduced threshold), then pop_back_val when traversing the Worklist above, resulting in a depth-first traversal of the call graph. What this means is that if we first encounter a call to a given external function foo() deep in a call chain, it will get added with a very low threshold. It may be that another external call to foo() is encountered higher up the call chain in something added to the worklist earlier and popped later, but since we skip any functions that were already seen (in the CalledFunctions set), we will miss the opportunity to add it with the higher threshold and possibly lose out on the import.

To fix this the tracking of what is or was already in the worklist needs to be more sophisticated. Perhaps a simple fix would be to change the CalledFunctions set to a map from function name to the max threshold it was added to the worklist with. Then if we encounter another call to the same function, if it currently has a higher threshold than recorded in the CalledFunctions map, add it to the worklist again and updated the map entry. Then each function popped off the worklist should be compared against the existing ModuleToFunctionsToImportMap entry to see if we have already decided to import it since it may end up in the worklist multiple times.


http://reviews.llvm.org/D17082





More information about the llvm-commits mailing list