[PATCH] D44847: [profile] Fix value profile runtime merging issues

[Rong Xu via llvm-commits]

One way to walk-around is to reset VPsites in Data. That would need to
change the APIs to
__llvm_profile_begin_data() to __llvm_profile_end_data() to remove the
const keyword

-Rong.

On Mon, Mar 26, 2018 at 11:35 AM Rong Xu <xur at google.com> wrote:

> My assumption is there that there can be value profiles when libc is not
> linked in.
> I have a test case to show the problem: this is just a a normal program
> with indirect calls but  with _llvm_profile_runtime set to 0. The program
> write the profile by itself.
>
>   I'm not sure if the use case where libc is not linked in.
>
> On Mon, Mar 26, 2018 at 11:30 AM Xinliang David Li <davidxl at google.com>
> wrote:
>
>>
>>
>>
>> On Mon, Mar 26, 2018 at 11:24 AM Rong Xu via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> xur added inline comments.
>>>
>>>
>>> ================
>>> Comment at: lib/profile/InstrProfilingFile.c:230
>>> +  // (which can leave garbage at the end of profile file).
>>> +  if (!VPMergeHook)
>>> +    truncate(ProfileFileName, 0);
>>> ----------------
>>> davidxl wrote:
>>> > xur wrote:
>>> > > davidxl wrote:
>>> > > > xur wrote:
>>> > > > > davidxl wrote:
>>> > > > > > non value profiling data has fixed size, so why is this needed?
>>> > > > > Because there are still value counts in both in-memory and
>>> in-disk profiles.
>>> > > > >
>>> > > > > Using the test in the patch as the example (running on my
>>> machine).
>>> > > > > assuming the binary is named as 'gen' and VPMergeHook null.
>>> > > > >
>>> > > > > if we don't do online merge,
>>> > > > > "gen" will generate a profile of size 320 bytes.
>>> > > > > "gen 1" will generate a profile with size of 352 bytes
>>> > > > >
>>> > > > > If we run "gen 1 && gen", the result binary is still 352 bytes.
>>> > > > > But the effective size is 320 bytes. The tail 32 bytes are trash.
>>> > > > >
>>> > > > >
>>> > > > Without VpMergeHook, we should assert that there is no value
>>> profile data -- it should not be dumped in the first place.
>>> > > current behavior is to write out the in-memory value profile data
>>> and discard the value profile data in the file. if there is not value
>>> profile from the file, the result is still correct.
>>> > >
>>> > > I'm not familiar with the use cast the not having libc (where
>>> VPMergeHook will be nullptr). Is it not suppose to have any value profile?
>>> > >
>>> > > I can assert VPMergeHook==nullptr and in-memory valuesites are not
>>> empty. That will be easy. Is this what you suggested?
>>> > In lprofGetVPDataReader method, returns null if VPMergeHook is null.
>>> This will generate incorrect profile: the valuesite in the data section
>>> says there are valuesites but VP count is empty.
>>>
>>>
>> We should assert that when VPMergeHook is null, the data section has no
>> value sites.  Do you see a possible case this does not hold?
>>
>> David
>>
>>
>>
>>
>>> if we do this, the behavior would be: llvm-profdata show or merge
>>> command will stop silently at the first function that has the value-site
>>> instrumentation. The rest profile will be discarded.
>>>
>>> Note, of course, this only applies to  VPMergeHook==nullptr case.
>>>
>>>
>>> https://reviews.llvm.org/D44847
>>>
>>>
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180326/15f44d90/attachment.html>