[PATCH] D48670: [ThinLTO] Ensure we always select the same function copy to import

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:224
+  // i.e. with weak or linkonce linkage).
+  auto *Summary = cast<FunctionSummary>((*It)->getBaseObject());
+  if (Summary->instCount() > Threshold)
----------------
tejohnson wrote:
> davidxl wrote:
> > This changes the behavior in the way that a callee that could be found before this patch may no longer be found. Is this expected?
> Yes it is a side-effect. We could potentially also change it to only check the prevailing copy (which is typically the first copy anyway), but that would also leave us sometimes without a copy to import whereas we might have imported a different smaller copy previously. 
> 
> Another possibility would be to keep a set of functions already imported in to the module and check it first. We do keep the imports currently, but it is by source module, so this would likely need another set to be faster.
How about fetch all the eligible candidates into a list, sort them by size and pick the first (smallest) one if it is below threshold. This  increases the chances an import will occur.


Repository:
  rL LLVM

https://reviews.llvm.org/D48670