[PATCH] D18343: ThinLTO: use the callgraph from the combined index to drive the FunctionImporter

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:95
@@ +94,3 @@
+/// null if there's no match.
+static const FunctionSummary *selectCallee(uint64_t GUID, unsigned Threshold,
+                                           const ModuleSummaryIndex &Index) {
----------------
joker.eph wrote:
> davidxl wrote:
> > joker.eph wrote:
> > > davidxl wrote:
> > > > Why not passing a reference of the Edge to 'selectCallee'. We will need to look at profile data in the future.
> > > I think the profile info will not impact *which* callee you select right? What do you expect to do with this information at this level?
> > It will impact importing strategy. For instance increased threshold for hot callsites and reduced threshold for extreme cold ones.  The summary Edge can also contain other nice bits  (such as has constant argument etc) which will help importing more precisely.
> I agree, it is not clear to me that exposing the "Edge" here is the right abstraction (one could modifying the Threshold at call site to include a PGO bias for instance, even if it is probably not the right thing either).
> 
> I think that I can safely leave this for the larger refactoring work that will follow when we will rework the cost model for importing. 
Sounds fine to me.


http://reviews.llvm.org/D18343