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

The profile summary is saved in the global metadata ASAIK. If we want to
calculate if something is hot/cold while choosing functions for importing,
we would either need to read whole Module (which we clearly don't want to
do)
or duplicate this information in the summary, so we could get it without
reading Module.

2016-09-02 15:49 GMT-07:00 Mehdi Amini <mehdi.amini at apple.com>:
>
> 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>:
>
>> On Fri, Sep 2, 2016 at 2:58 PM, Piotr Padlewski
>> <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/3a631bf9/attachment-0001.html>

> On Sep 2, 2016, at 4:13 PM, Piotr Padlewski <piotr.padlewski at
gmail.com> wrote:
> 
> The profile summary is saved in the global metadata ASAIK. If we want to
calculate if something is hot/cold while choosing functions for importing, we
would either need to read whole Module (which we clearly don't want to do)
> or duplicate this information in the summary, so we could get it without
reading Module.
When I say “explain a bit more” I meant:

- what is the profile summary?
- how do you compute the hot/cold information from it and from the count we
attache to edges?

— 
Mehdi
> 
> 2016-09-02 15:49 GMT-07:00 Mehdi Amini <mehdi.amini at apple.com
<mailto:mehdi.amini at apple.com>>:
> 
>> On Sep 2, 2016, at 3:16 PM, Piotr Padlewski <piotr.padlewski at
gmail.com <mailto: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/8ef24384/attachment.html>

2016-09-02 16:55 GMT-07:00 Mehdi Amini <mehdi.amini at apple.com>:
>
> On Sep 2, 2016, at 4:13 PM, Piotr Padlewski <piotr.padlewski at
gmail.com>
> wrote:
>
> The profile summary is saved in the global metadata ASAIK. If we want to
> calculate if something is hot/cold while choosing functions for importing,
> we would either need to read whole Module (which we clearly don't want
to
> do)
> or duplicate this information in the summary, so we could get it without
> reading Module.
>
>
> When I say “explain a bit more” I meant:
>
> Sure, I didn't know which part is not clear:
> - what is the profile summary?
>
Profile summary is additional metadata in LLVM calculated from FDO/aFDO
data.
It is basically ProfileSummary class, having vector of ProfileSummaryEntry.

> - how do you compute the hot/cold information from it and from the count
> we attache to edges?
>
> Having the ProfileCount from CalleeInfo, we can know what is the hotness
of callsite (of the block).
 - or at least the average hotness of basic blocks that calls give callee.
(We probably want to change it
to std::max of callsite hotness later)
Then having this information we can call ProfileSummaryInfo::isHotCount()
to find out if the callsite is hot,
which would probably mean that we want to inline callee to this basic block.
Right now Inliner only cares about the callee hotness, not callsite
hotness, but this will change when
we will have new pass manager (the Easwaran patch depends on it).

Is that enough information? Fell free to ask some more, or ping me directly
Mehdi.

Piotr

—
> Mehdi
>
>
> 2016-09-02 15:49 GMT-07:00 Mehdi Amini <mehdi.amini at apple.com>:
>
>>
>> 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>:
>>
>>> On Fri, Sep 2, 2016 at 2:58 PM, Piotr Padlewski
>>> <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/b671ecef/attachment.html>