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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

2



================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:330
+    bool IsHotCallsite = Edge.second.Hotness == CalleeInfo::HotnessType::Hot;
+    const auto AdjThreshold = GetAdjustedThreshold(Threshold, IsHotCallsite);
+
----------------
mehdi_amini wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > I'm fairly lost in the logic here between `GetAdjustedThreshold` and `GetBonusMultiplier`, still trying to make sense of what we're currently doing here.
> > OK I figured what's going on, it is quite obscure to infer from the code though, and I believe the comment above is misleading.
> (I take back what I wrote for the comment, was looking at the wrong place... So confusing!)
This code motion to use `AdjThreshold` instead of `Threshold` below seems like a standalone fix?


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:343
     // Mark this function as imported in this module, with the current Threshold
-    ProcessedThreshold = Threshold;
+    ProcessedThreshold = AdjThreshold;
 
----------------
All this logic to skip inserting in the worklist seems redundant with the new logic you added below while iterating over the work list (the latter seems to be a superset).

 


https://reviews.llvm.org/D27696