[PATCH] D23106: [Profile] introduce interface __llvm_profile_dump

[Xinliang David Li via llvm-commits]

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
>
>
>