[PATCH] D23106: [Profile] introduce interface __llvm_profile_dump

[Vedant Kumar via llvm-commits]

Makes sense. LGTM.

thanks,
vedant

> On Aug 8, 2016, at 10:46 AM, Xinliang David Li <davidxl at google.com> wrote:
> 
> On Mon, Aug 8, 2016 at 10:38 AM, Vedant Kumar <vsk at apple.com> wrote:
>> vsk added inline comments.
>> 
>> ================
>> Comment at: lib/profile/InstrProfiling.c:81
>> @@ -70,2 +80,3 @@
>>   }
>> +  ProfileDumped = 0;
>> }
>> ----------------
>> davidxl wrote:
>>> vsk wrote:
>>>> Should probably also be cleared when llvm_prof_set_filename() or llvm_profile_merge_from_buffer() are called.
>>> It is a mistake to not call reset counter before dumping again (due to double counting) regardless whether merging is used or not
>> Oh, I see. This sequence of calls doesn't make sense: dump(), merge_profile_from_buffer(), dump(). You're saying you'd need to reset the counters some time before the second call to dump() anyway.
>> 
>> What about: dump(), set_filename(), dump()?. The second 'dump()' wouldn't go through, which seems like a problem.
>> 
> 
> This is usually
> 
> region-1 --> dump() --> code region-2 --> set_filename --> dump()
> 
> so user wants to dump region-1 profile in one profile, and region1+
> region2 to second file.  It does not bring user any benefit by doing
> this. Forcing resetting allows file2 only contains region-2 and post
> process can be done to merge two regions.  If user really wants this,
> he always has the choice of  using low level interface write_file
> directly.    Enforcing the resetting between dumps makes the use model
> cleaner.
> 
> David
> 
> 
> 
>> 
>> https://reviews.llvm.org/D23106
>> 
>> 
>>