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

[Piotr Padlewski via llvm-dev]

2016-09-02 17:50 GMT-07:00 Piotr Padlewski <piotr.padlewski at gmail.com>:

>
>
> 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)
>
 Sorry, this problem will go away if we get rid of the ProfileCount from
CalleeInfo.


> 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/928d7864/attachment.html>