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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson 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)
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D48670