<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 11, 2015 at 6:51 PM, Betul Buyukkurt <span dir="ltr"><<a href="mailto:betulb@codeaurora.org" target="_blank">betulb@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> davidxl added inline comments.<br>
><br>
> ================<br>
> Comment at: lib/profile/InstrProfiling.c:175<br>
> @@ +174,3 @@<br>
> +<br>
> +    for (uint32_t i = 0; i < NumVSites; ++i) {<br>
> +<br>
> ----------------<br>
> Looks like the structure is very close to ValueProfData structure used in<br>
> indexed format (except that the later is organized per-value kind, while<br>
> here it is combined).  It will get all the interfaces for reading etc.<br>
<br>
</span>So, is this another file format change cycle? We've already changed the<br>
format three times during the life time of this CL.<br>
<span class=""><br></span></blockquote><div><br></div><div>The two formats are basically the same, so there is no much change essentially. See my comment in another patch, it is more about code sharing and reuse. Having said that, I am fine with changing this as a follow up.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> ================<br>
> Comment at: lib/profile/InstrProfilingBuffer.c:45<br>
> @@ -41,3 +44,3 @@<br>
>        PROFILE_RANGE_SIZE(Counters) * sizeof(uint64_t) +<br>
> -      NamesSize + Padding;<br>
> +      NamesSize + Padding2;<br>
>  }<br>
> ----------------<br>
> Value Profile Data size is not counted here.<br>
<br>
</span>We do not make use of these API's, thus we first need input from the<br>
community (specifically the users of these API's) to change them.<br>
Currently, VP is just not supported w/ the Buffer API's. In case buffer<br>
API's are used to retrieve profile data during an instrumented executable<br>
continues it's execution (jit like scenarios?), then updating the<br>
ValueCounters member of __llvm_profile_data would possibly break the<br>
profiler.<br>
<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>This does not sound right -- buffer API and file API now behaves differently.  The right way to do is:</div><div><br></div><div>1) introduce a lock for dumping. When the dumping lock is help, the profiler will simply return without updating</div><div>2) when dumping is finished, the valueCounters can be cleared to be null (and edge profile count needs to be cleared too) and the lock can be released.</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="HOEnZb"><font color="#888888">
-Betul<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> ================<br>
> Comment at: lib/profile/InstrProfilingBuffer.c:110<br>
> @@ -101,2 +109,3 @@<br>
> +  UPDATE_memcpy(Zeroes,        Padding2      * sizeof(char));<br>
>  #undef UPDATE_memcpy<br>
><br>
> ----------------<br>
> Value profile data is missing here.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D9009" rel="noreferrer" target="_blank">http://reviews.llvm.org/D9009</a><br>
><br>
><br>
><br>
><br>
<br>
<br>
</div></div></blockquote></div><br></div></div>