[LLVMdev] IC profiling infrastructure

[Betul Buyukkurt]

Ping?

-----Original Message-----
From: Betul Buyukkurt [mailto:betulb at codeaurora.org] 
Sent: Thursday, June 25, 2015 11:47 AM
To: Xinliang David Li
Cc: Justin Bogner; Betul Buyukkurt; llvmdev
Subject: Re: [LLVMdev] IC profiling infrastructure

> Justin, do you have more concerns on keeping value_kind?

Hi Justin,

The next CL's according to the merge plan all depend on the value_kind.
Therefore both http://reviews.llvm.org/D10471 and
http://reviews.llvm.org/D10674 introduce it. Could you please review these
CL's?

As David stated, we need to bring the value_kind discussion to finalization
w/ a decision. Since the original IC profiling patches to today it's been
over two months and we're still not arrived at a conclusion.

-Betul

> If there is still disagreement, can we agree to move on with it for 
> now ?  After the initial version of the patches checked in, we can do 
> more serious testings with large apps and revisit this if there are 
> problems discovered.
>
> thanks,
>
> David
>
>
>
> On Mon, Jun 15, 2015 at 10:47 PM, Xinliang David Li 
> <davidxl at google.com>
> wrote:
>>> But since the consumer is the frontend, and it knows which counts 
>>> are which, it knows the kind, no? I don't understand how storing the 
>>> kind info helps or hurts - it's just redundant.
>>
>> Yes, the frontend consumer knows, but the raw reader does not. It is 
>> natural to do those processing when processing the raw profile data 
>> (for instance when function pointer to name mapping still exists).
>>
>>>
>>>>>
>>>>> The other thing we should do is store which profiling options are 
>>>>> enabled, in both formats. When we specify profile-instr-use it'll 
>>>>> be less error prone if we can detect whether or not this includes 
>>>>> indirect call profiling without checking other options, and it's 
>>>>> probably a good idea for "llvm-profdata merge" to disallow merging 
>>>>> profiles that are gathering different sets of data. A bitfield 
>>>>> seems suitable for this.
>>>>
>>>> For value profiling, allowing profile-gen and profile-use passes 
>>>> using different options is a useful feature. Consider the following
>>>> scenarios:
>>>> 1) collect the same profile data once with all kinds of value profile
>>>>    data collected. The exact same profile data can be used in 
>>>> performance
>>>>    experiments with different kinds of value profiling 
>>>> enabled/disabled
>>>
>>> This use case is pretty easy to accomodate for with the model where 
>>> the profile stores its type. We could autodetect the type to decide 
>>> how to interpret the profile, and if more specific profile flags are 
>>> set simply ignore some of the data. The thing that the file format 
>>> storing the type gives us is that it's harder to misinterpret the 
>>> data by supplying the mismatching flags between reading and writing.
>>
>> Yes, this is achievable with some extra effort -- for instance by 
>> collecting the profile sites of all kinds regardless of the flags.
>> After profiling matching, simply drop selected sites according to the 
>> flags.
>>
>> This adds complexity of the code. With value kinds encoded in profile 
>> data, the handling is much simpler -- each value profiler only needs 
>> to deal with its own kind.
>>
>>>
>>>> 2) work around profile-use/value transformation bug selectively for
>>>>    some file without the need to change anything in instrumentation 
>>>> pass
>>>
>>> I'm not sure I understand what you mean here.
>>
>> Similar to the above, but for correctness.
>>
>>>
>>>> Besides, with the latest patch, the value_kind is not recorded in 
>>>> each profile value thus the overhead is minimized.
>>>
>>> The overhead is low, sure, but the code complexity of dealing with 
>>> the multiple kinds in this way is quite real.
>>
>> I actually believe having value-kind can help reduce code complexity 
>> instead of the other way around. What is the extra complexity?
>>
>> thanks,
>>
>> David
>>
>>
>>>Since I'm not convinced we're
>>> getting real benefit from storing the kind, I don't believe this  
>>>trade-off is worth it.
>>>
>>>> thanks,
>>>>
>>>> David
>