[PATCH] D9009: Value profiling compiler-rt changes

[Betul Buyukkurt via llvm-commits]

> davidxl added inline comments.
>
> ================
> Comment at: lib/profile/InstrProfiling.c:81
> @@ +80,3 @@
> +void __llvm_profile_instrument_target(uint64_t TargetValue, void *Data_,
> +  uint32_t CounterIndex) {
> +
> ----------------
> Can we make the CounterIndex relative to the value kind? Why is value kind
> parameter not passed in?

It's not passed in, because it's already relative to the value kind.
CounterIndex is computed inside the InstrProfiling.cpp of the llvm patch
during lowering. It's a value that can be computed in compile time, so
better to do it then.

-Betul

>
> ================
> Comment at: lib/profile/InstrProfiling.c:87
> @@ +86,3 @@
> +
> +  if (!Data->ValueCounters) {
> +    uint64_t NumVSites = 0;
> ----------------
> This part of the code is cold -- move it out of line into another routine.
>
> ================
> Comment at: lib/profile/InstrProfiling.c:119
> @@ +118,3 @@
> +
> +  if (VDataCount >= UCHAR_MAX)
> +    return;
> ----------------
> This may still be too large. If VDataCount is this large, the runtime
> overhead will be substantial.
>
> In the future, we should enhance it with the following algoithm:
> 1) reserve the first vdata node to do book keeping -- i.e. tracking number
> of time values get dropped due to too long a list
> 2) When the number of dropping times reaches a threshold, sort the
> existing
> linked list to make it in descending order, and zero out the last X
> entries.
>
> Not needed for the first version.
>
> ================
> Comment at: lib/profile/InstrProfiling.h:33
> @@ +32,3 @@
> +  VK_FIRST = VK_IndirectCallTarget,
> +  VK_LAST = VK_IndirectCallTarget
> +} __llvm_profile_value_kind;
> ----------------
> Make the enumerators name the same as the LLVM counter part -- this allows
> future refactoring.
>
>
> http://reviews.llvm.org/D9009
>
>
>
>