[PATCH] D9009: Value profiling compiler-rt changes

[Betul Buyukkurt via llvm-commits]

betulb added a comment.

> davidxl added inline comments.

> 

> ================

>  Comment at: lib/profile/InstrProfiling.c:175

>  @@ +174,3 @@

>  +

>  +    for (uint32_t i = 0; i < NumVSites; ++i) {

>  +

> 

>  ----------------

> 

> Looks like the structure is very close to ValueProfData structure used in

>  indexed format (except that the later is organized per-value kind, while

>  here it is combined).  It will get all the interfaces for reading etc.


So, is this another file format change cycle? We've already changed the
format three times during the life time of this CL.

> ================

>  Comment at: lib/profile/InstrProfilingBuffer.c:45

>  @@ -41,3 +44,3 @@

> 

>   PROFILE_RANGE_SIZE(Counters) * sizeof(uint64_t) +

> 

> - NamesSize + Padding; +      NamesSize + Padding2; } ---------------- Value Profile Data size is not counted here.


We do not make use of these API's, thus we first need input from the
community (specifically the users of these API's) to change them.
Currently, VP is just not supported w/ the Buffer API's. In case buffer
API's are used to retrieve profile data during an instrumented executable
continues it's execution (jit like scenarios?), then updating the
ValueCounters member of __llvm_profile_data would possibly break the
profiler.

-Betul

> ================

>  Comment at: lib/profile/InstrProfilingBuffer.c:110

>  @@ -101,2 +109,3 @@

>  +  UPDATE_memcpy(Zeroes,        Padding2      * sizeof(char));

> 

>   #undef UPDATE_memcpy

> 

>  ----------------

> 

> Value profile data is missing here.

> 

> http://reviews.llvm.org/D9009



http://reviews.llvm.org/D9009