[PATCH] Indirect call target profiling compiler-rt changes

[Xinliang David Li]

On Tue, Jun 2, 2015 at 9:34 AM, Betul Buyukkurt <betulb at codeaurora.org> wrote:
>> I think there is more room to reduce memory footprint and profile data
> size.
>> The reason that value_kind and counter index needs to be recorded and
> stored is because the number of value entries per site is variant. This
> problem can be solved by adding one level of indirection when writing
> out
>> the profile data: Writing out ValueCounters in profile data too (with
> address fixup after data gathering). With that written out, the
>> llvm_profile_value data can be greatly shrinked:
>
> Hi David,
>
> Here is what you're suggesting per my understanding.
>
> In the raw profile output file, ValueCounters array of __llvm_profile_data
> should point to another array of size NumValueSites. This array is created
> dynamically at runtime, but will also be written out to the profile data
> file. Each element of this new array in the output file will point to the
> beginning of the relevant __llvm_profile_value_data* entries for a
> value_kind and a counter_index.

yes. It basically makes value profile data representation more
consistent with branch profile  data -- the new array basically maps
to the counter array of the branch profile.


>
> __llvm_profile_gather_value_data routine will be responsible for forming
> both the __llvm_profile_value_data* array (i.e. ValueProfilingResults) as
> well as the new pointer array for the added indirection.

yes.

>
> One extra piece of information that needs to be stored is the number of
> elements that each pointer in the indirection array points to.

No need -- it can be computed by diffing the value of pointer of
current counter index with the next entry.

> In the
> least we have to store the number of elements that the last element in the
> indirection array points to. This way we may eliminate the need to store
> the counter index, function index and the value_kinds in each value data
> field.

No need for that. What is needed is just a sentinel element in the
indirection array that points to the position after the end of the
value entries. Basically, the number of elements of the indirect array
is N+1 where N is the number of sites of that value kind.

> Please confirm if anything is missing from the above. Since the raw
> profile format does not have to be maintained and remain backwards
> compatible (per Justin's comments), this optimization can be added after
> all the patches merge in. So this should not really hold the patches from
> merging in. Do you agree?

True, but I prefer not going that route if possible:

1)  Getting it right the first time can avoid the rework and save us a
lot of time in the future;
2)  You have done a great work so far ;) It will probably take you
much less time to do this than others ..

thanks,

David

>
> -Betul
>
>> 1. in memory: it needs only 3 64bit integers instead of 4
>> 2. in disk: it needs only 2 64bit integers instead of the current 4 (No
> need for func index either).
>> http://reviews.llvm.org/D9009
>> EMAIL PREFERENCES
>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
>