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

[Rong Xu via llvm-commits]

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