[PATCH] D27696: [ThinLTO] Thin link efficiency: skip candidate added later with higher threshold (NFC)

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added a reviewer: mehdi_amini.
tejohnson added a subscriber: llvm-commits.

Another thin link efficiency improvement. After adding an importing
candidate to the worklist we might have later added it again with a
higher threshold. Skip it when popped from the worklist if we recorded a
higher threshold than the current worklist entry, it will get processed
again at the higher threshold when that entry is popped.

This required 2 changes:

- add the summary's GUID to the worklist, so that it can be used to query the recorded highest threshold for it when we pop from the worklist.
- Keep the same adjusted threshold in both the worklist and the ImportList, so that they can be compared. This means computing the adjusted threshold (previously only added to the worklist) earlier, and using it in both the ImportList entry and comparison.


https://reviews.llvm.org/D27696

Files:
  lib/Transforms/IPO/FunctionImport.cpp


Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -266,7 +266,8 @@
   ExportList.insert(GUID);
 }
 
-using EdgeInfo = std::pair<const FunctionSummary *, unsigned /* Threshold */>;
+using EdgeInfo = std::tuple<const FunctionSummary *, unsigned /* Threshold */,
+                            GlobalValue::GUID>;
 
 /// Compute the list of functions to import for a given caller. Mark these
 /// imported functions and the symbols they reference in their source module as
@@ -316,18 +317,30 @@
     assert(ResolvedCalleeSummary->instCount() <= NewThreshold &&
            "selectCallee() didn't honor the threshold");
 
+    auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) {
+      // Adjust the threshold for next level of imported functions.
+      // The threshold is different for hot callsites because we can then
+      // inline chains of hot calls.
+      if (IsHotCallsite)
+        return Threshold * ImportHotInstrFactor;
+      return Threshold * ImportInstrFactor;
+    };
+
+    bool IsHotCallsite = Edge.second.Hotness == CalleeInfo::HotnessType::Hot;
+    const auto AdjThreshold = GetAdjustedThreshold(Threshold, IsHotCallsite);
+
     auto ExportModulePath = ResolvedCalleeSummary->modulePath();
     auto &ProcessedThreshold = ImportList[ExportModulePath][GUID];
     /// Since the traversal of the call graph is DFS, we can revisit a function
     /// a second time with a higher threshold. In this case, it is added back to
     /// the worklist with the new threshold.
-    if (ProcessedThreshold && ProcessedThreshold >= Threshold) {
+    if (ProcessedThreshold && ProcessedThreshold >= AdjThreshold) {
       DEBUG(dbgs() << "ignored! Target was already seen with Threshold "
                    << ProcessedThreshold << "\n");
       continue;
     }
     // Mark this function as imported in this module, with the current Threshold
-    ProcessedThreshold = Threshold;
+    ProcessedThreshold = AdjThreshold;
 
     // Make exports in the source module.
     if (ExportLists) {
@@ -348,20 +361,8 @@
       }
     }
 
-    auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) {
-      // Adjust the threshold for next level of imported functions.
-      // The threshold is different for hot callsites because we can then
-      // inline chains of hot calls.
-      if (IsHotCallsite)
-        return Threshold * ImportHotInstrFactor;
-      return Threshold * ImportInstrFactor;
-    };
-
-    bool IsHotCallsite = Edge.second.Hotness == CalleeInfo::HotnessType::Hot;
-
     // Insert the newly imported function to the worklist.
-    Worklist.emplace_back(ResolvedCalleeSummary,
-                          GetAdjustedThreshold(Threshold, IsHotCallsite));
+    Worklist.emplace_back(ResolvedCalleeSummary, AdjThreshold, GUID);
   }
 }
 
@@ -395,8 +396,16 @@
   // Process the newly imported functions and add callees to the worklist.
   while (!Worklist.empty()) {
     auto FuncInfo = Worklist.pop_back_val();
-    auto *Summary = FuncInfo.first;
-    auto Threshold = FuncInfo.second;
+    auto *Summary = std::get<0>(FuncInfo);
+    auto Threshold = std::get<1>(FuncInfo);
+    auto GUID = std::get<2>(FuncInfo);
+
+    // Check if we later added this summary with a higher threshold.
+    // If so, skip this entry.
+    auto ExportModulePath = Summary->modulePath();
+    auto &LatestProcessedThreshold = ImportList[ExportModulePath][GUID];
+    if (LatestProcessedThreshold > Threshold)
+      continue;
 
     computeImportForFunction(*Summary, Index, Threshold, DefinedGVSummaries,
                              Worklist, ImportList, ExportLists);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27696.81175.patch
Type: text/x-patch
Size: 3770 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/e6c0dc30/attachment.bin>