[llvm-dev] [ThinLTO] Importing based on PGO data

[Mehdi Amini via llvm-dev]

> On Sep 2, 2016, at 3:16 PM, Piotr Padlewski <piotr.padlewski at gmail.com> wrote:
> 
> 
> 
> 2016-09-02 15:04 GMT-07:00 Xinliang David Li <davidxl at google.com <mailto:davidxl at google.com>>:
> On Fri, Sep 2, 2016 at 2:58 PM, Piotr Padlewski
> <piotr.padlewski at gmail.com <mailto:piotr.padlewski at gmail.com>> wrote:
> > Hi,
> > I am working right now on importing based on PGO/FDO data. There is one
> > issue that I found - when we calculate the list of imports, we can't get the
> > ProfileSummaryInfo, which is the best and I
> > think only valid way of checking if callsite/callee is hot (isHotCount()).
> > There are 2 solutions that I come up with Teresa and Easwaran:
> >
> > 1. Add PGO data to summary
> > 2. Replace CalleeInfo::ProfileCount with enum {None, Cold, Hot} computed
> > during computing summary.
> 
> 
> Don't we already have edge profile count in the callgraph summary?
> I think what is missing is the Profile SUmmary data itself -- that one
> should be copied over to thinLTO summary so that the importing
> analysis can use. However I we should not need to duplicate the
> information in every module.
> 
> David
> 
> Yes we do have edge profile cout, but in order to compare it with global couts we need Profile Summary as you said. 

Can you explain a bit more the issue here?

Thanks,

Mehid


> If we will follow 2) then we won't have to duplicate the data.
>  
> >
> > I like the 2. much more. It will reduce the summary size slightly and I
> > don't think we will need ProfileCount anywhere else.
> >
> > The other thing I would like to mention is that I think we should start
> > using the summary versioning and drop support of old version.
> > ThinLTO doesn't have enough users right now and parsing many versions of
> > summary will just add additional cost, that will start to grow.
> >
> > Piotr

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160902/c4d2fff4/attachment.html>