[PATCH] D28964: [PGO] Value profile support for value ranges

[Rong Xu via llvm-commits]

On Tue, Jan 24, 2017 at 11:15 AM, Xinliang David Li <davidxl at google.com>
wrote:

>
>
> On Tue, Jan 24, 2017 at 11:13 AM, Rong Xu <xur at google.com> wrote:
>
>>
>>
>> On Tue, Jan 24, 2017 at 11:02 AM, Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Jan 24, 2017 at 10:57 AM, Rong Xu <xur at google.com> wrote:
>>>
>>>> Yes. that's another way to do this. I considered that when I
>>>> implemented the code. I choose not do because:
>>>> (1) it makes it the instrumentation code complex and increase the code
>>>> size as basically you inline the logic to every instrumentation site.
>>>>
>>>
>>> We can filter a lot of memcpy sites with constant sizes, so the number
>>> of sites remaining should not be large (compared to virtual calls).
>>>
>> in real applications, the static call site for memcpy and memset is not
>> small.
>>
>>>
>>>
>>>> (2) it affects the static counter allocation. This range profile
>>>> potentially uses a lot more counters that current indirectly calls.
>>>>
>>>
>>> Why is that?
>>>
>> we collect the precise size for small sized memcpy and memset. Most of
>> the callsite has multiple size values.
>>
>
> My question is how is this affected by the way it is implemented: 1) using
> API call ; 2) generating inline sequence of interval profiling.
>

using API, I have the exact count of the range instrumentations (and the
range) so I can adjust the counter allocation in lowering (different from
indirect calls)
On the other hand, if we do the inline sequence in instrumentation, we
treat them the same as current indirect calls instrumentation.


>
> Anyway, see my previous reply -- I think having a minimal wrapper API in
> compiler rt is fine.
>
> David
>
>
>>
>>> David
>>>
>>>
>>>
>>>
>>>> The advantage of not change compiler_rt is that less information is
>>>> needed to pass b/w runtime and instrumentation. This especially true is we
>>>> want to have profile client to control it's own ranges.
>>>>
>>>> But I still think the change in compiler_rt is straight-forward way.
>>>>
>>>> On Tue, Jan 24, 2017 at 10:33 AM, David Li via Phabricator <
>>>> reviews at reviews.llvm.org> wrote:
>>>>
>>>>> davidxl added a comment.
>>>>>
>>>>> In fact, I don't there is a need for compiler_rt change at all for the
>>>>> interval/range profiling. The compiler_rt API should be kept simple.  In
>>>>> other words, interval profiling should already be 'lowered' into regular
>>>>> value profiling in the instrumentation lowering phase. The instrumentation
>>>>> lowering will general inline sequence that collapse ranges into single
>>>>> value depending on the value profile kind.  Only the
>>>>> __llvm_profile_instrument_target low level interface is needed.
>>>>>
>>>>> The only downside of this approach that we can not override the range
>>>>> definitions at runtime -- but I don't think that is worth the complexity
>>>>> introduced.
>>>>>
>>>>>
>>>>> Repository:
>>>>>   rL LLVM
>>>>>
>>>>> https://reviews.llvm.org/D28964
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170124/91bbe0be/attachment.html>