[PATCH] Indirect call target profiling compiler-rt changes

Justin Bogner mail at justinbogner.com
Thu May 21 16:20:30 PDT 2015


Betul Buyukkurt <betulb at codeaurora.org> writes:
> New fields are reserved in enum ValueKind { ... } for future value
> profiling types.
>
>
> http://reviews.llvm.org/D9009
>
> Files:
>   lib/profile/InstrProfiling.c
>   lib/profile/InstrProfiling.h
>   lib/profile/InstrProfilingBuffer.c
>   lib/profile/InstrProfilingFile.c
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> Index: lib/profile/InstrProfiling.c
> ===================================================================
> --- lib/profile/InstrProfiling.c
> +++ lib/profile/InstrProfiling.c
> @@ -8,6 +8,7 @@
>  \*===----------------------------------------------------------------------===*/
>
>  #include "InstrProfiling.h"
> +#include <stdlib.h>
>  #include <string.h>
>
>  __attribute__((visibility("hidden")))
> @@ -24,7 +25,7 @@
>    unsigned char R = sizeof(void *) == sizeof(uint64_t) ? 'r' : 'R';
>    return
>      (uint64_t)255 << 56 |
> -    (uint64_t)'l' << 48 |
> +    (uint64_t)'v' << 48 |
>      (uint64_t)'p' << 40 |
>      (uint64_t)'r' << 32 |
>      (uint64_t)'o' << 24 |

Not necessary.

> @@ -36,13 +37,154 @@
>  __attribute__((visibility("hidden")))
>  uint64_t __llvm_profile_get_version(void) {
>    /* This should be bumped any time the output format changes. */
> -  return 1;
> +  return 2;
>  }
>
>  __attribute__((visibility("hidden")))
>  void __llvm_profile_reset_counters(void) {
>    uint64_t *I = __llvm_profile_begin_counters();
>    uint64_t *E = __llvm_profile_end_counters();
>
>    memset(I, 0, sizeof(uint64_t)*(E - I));
> +
> +  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
> +  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
> +  for (const __llvm_profile_data *DI = DataBegin; DI != DataEnd; ++DI) {
> +
> +    for (uint32_t VKI = VK_FIRST; VKI != VK_LAST; ++VKI) {
> +
> +      if (!DI->ValueCounters[VKI])
> +        continue;
> +
> +      for (uint32_t i = 0; i < DI->NumValueSites[VKI]; ++i) {
> +        __llvm_profile_value_data *CurrentValueData = DI->ValueCounters[VKI][i];
> +
> +        while (CurrentValueData) {
> +          CurrentValueData->NumTaken = 0;
> +          CurrentValueData = CurrentValueData->Next;
> +        }
> +      }
> +    }
> +  }
> +}
> +
> +__attribute__((visibility("hidden")))
> +void __llvm_profile_instrument_target(uint32_t ValueKind, uint64_t TargetValue,
> +  void *Data_, uint32_t CounterIndex) {
> +
> +  __llvm_profile_data *Data = (__llvm_profile_data*)Data_;
> +  if (!Data)
> +    return;
> +
> +  if (0 >= Data->NumValueSites[ValueKind])
> +    return;
> +
> +  if (CounterIndex >= Data->NumValueSites[ValueKind])
> +    return;
> +
> +  if (!Data->ValueCounters[ValueKind]) {
> +    __llvm_profile_value_data** Mem = (__llvm_profile_value_data**)
> +      calloc(Data->NumValueSites[ValueKind], sizeof(__llvm_profile_value_data*));
> +    if (!Mem)
> +      return;
> +    if (!__sync_bool_compare_and_swap(
> +        &(Data->ValueCounters[ValueKind]), 0, Mem)) {
> +      free(Mem);
> +      return;
> +    }
> +  }
> +
> +  __llvm_profile_value_data *PrevValueData = NULL;
> +  __llvm_profile_value_data *CurrentValueData =
> +    Data->ValueCounters[ValueKind][CounterIndex];
> +
> +  while (CurrentValueData) {
> +    if (TargetValue == CurrentValueData->TargetValue) {
> +      CurrentValueData->NumTaken++;
> +      return;
> +    }
> +    PrevValueData = CurrentValueData;
> +    CurrentValueData = CurrentValueData->Next;
> +  }
> +
> +  CurrentValueData = (__llvm_profile_value_data*)
> +    calloc(1, sizeof(__llvm_profile_value_data));
> +  if (!CurrentValueData)
> +    return;
> +
> +  CurrentValueData->ValueKind = ValueKind;
> +  CurrentValueData->TargetValue = TargetValue;
> +  CurrentValueData->CounterIndex = CounterIndex;
> +  CurrentValueData->NumTaken++;
> +
> +  uint32_t Success = 0;
> +  if (!Data->ValueCounters[ValueKind][CounterIndex])
> +   Success = __sync_bool_compare_and_swap(
> +      &(Data->ValueCounters[ValueKind][CounterIndex]), 0, CurrentValueData);
> +  else if (PrevValueData && !PrevValueData->Next)
> +    Success = __sync_bool_compare_and_swap(
> +      &(PrevValueData->Next), 0, CurrentValueData);
> +
> +  if (!Success) {
> +    free(CurrentValueData);
> +    return;
> +  }
> +
> +  TotalValueDataCount += Success;

Do we need to update this count atomically?

>  }
> +
> +__attribute__((visibility("hidden")))
> +uint64_t __llvm_profile_gather_value_data(
> +  __llvm_profile_value_data** ValueProfilingResults) {
> +
> +  if (!ValueProfilingResults)
> +    return 0;
> +
> +  if (0 == TotalValueDataCount)
> +    return 0;
> +
> +  uint64_t NumDataCount = TotalValueDataCount;
> +  *ValueProfilingResults = (__llvm_profile_value_data*)
> +    calloc(NumDataCount, sizeof(__llvm_profile_value_data));
> +
> +  if (!*ValueProfilingResults)
> +    return 0;
> +
> +  uintptr_t CurrentFunctionIndex = -1;
> +  unsigned CurrentResultIndex = 0;
> +  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
> +  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
> +  for (const __llvm_profile_data *I = DataBegin; I != DataEnd; ++I) {
> +
> +    CurrentFunctionIndex++;
> +
> +    for (uint32_t VKI = VK_FIRST; VKI != VK_LAST; ++VKI) {
> +
> +      if (!I->ValueCounters[VKI])
> +        continue;
> +
> +      for (unsigned i = 0; i < I->NumValueSites[VKI]; ++i) {
> +
> +        __llvm_profile_value_data *CurrentValueData = I->ValueCounters[VKI][i];
> +
> +        while (CurrentValueData) {
> +          (*ValueProfilingResults)[CurrentResultIndex] = *CurrentValueData;
> +
> +          // This assignment will only be read by llvm-profdata which knows
> +          // that this value is no longer a pointer but an integer index to
> +          // the __llvm_profile_data struct.
> +          // The casting below is there to suppress the warning.
> +          (*ValueProfilingResults)[CurrentResultIndex].Next =
> +            (__llvm_profile_value_data*)CurrentFunctionIndex;
> +
> +          if (++CurrentResultIndex >= NumDataCount)
> +            return NumDataCount;

This is an error condition, right? Should we return 0 here like we do
for other errors? I'm worried about silently giving invalid data.

> +          CurrentValueData = CurrentValueData->Next;
> +        }
> +      }
> +    }
> +  }
> +  return NumDataCount;
> +}
> +
> +
> Index: lib/profile/InstrProfiling.h
> ===================================================================
> --- lib/profile/InstrProfiling.h
> +++ lib/profile/InstrProfiling.h
> @@ -27,14 +27,35 @@
>
>  #endif /* defined(__FreeBSD__) && defined(__i386__) */
>
> -#define PROFILE_HEADER_SIZE 7
> +#define PROFILE_HEADER_SIZE 9
> +
> +typedef enum ValueKind {
> +  VK_FIRST = 0,
> +  VK_IndirectCallTarget = 0,
> +  VK_Reserved1 = 1,
> +  VK_Reserved2 = 2,
> +  VK_LAST = 3
> +} __llvm_profile_value_kind;
> +
> +uint64_t TotalValueDataCount;

This variable probably belongs in InstrProfiling.c.

> +
> +typedef struct __llvm_profile_value_data {
> +  uint64_t TargetValue;
> +  uint64_t NumTaken;
> +  uint32_t ValueKind;
> +  uint32_t CounterIndex;
> +  struct __llvm_profile_value_data *Next;
> +} __llvm_profile_value_data;
>
>  typedef struct __llvm_profile_data {
>    const uint32_t NameSize;
>    const uint32_t NumCounters;
>    const uint64_t FuncHash;
>    const char *const Name;
>    uint64_t *const Counters;
> +  const uint8_t *FunctionPointer;

Is this used?

> +  const uint32_t NumValueSites[VK_LAST];
> +  __llvm_profile_value_data **ValueCounters[VK_LAST];
>  } __llvm_profile_data;
>
>  /*!
> @@ -58,6 +79,23 @@
>  uint64_t *__llvm_profile_end_counters(void);
>
>  /*!
> + * \brief Counts the number of times a target value of ValueKind is seen.
> + *
> + * Records the target value for a given ValueKind if not seen before. Otherwise,
> + * increments the counter associated w/ the target value.
> + */
> +void __llvm_profile_instrument_target(uint32_t ValueKind,
> +  uint64_t TargetValue, void *Data_, uint32_t CounterIndex);
> +
> +/*!
> + * \brief Prepares the value profiling data for output.
> + *
> + * Prepares the value profiling data into a single array.
> + */
> +uint64_t __llvm_profile_gather_value_data(
> +  __llvm_profile_value_data** ValueProfilingResults);
> +
> +/*!
>   * \brief Write instrumentation data to the current file.
>   *
>   * Writes to the file with the last name given to \a __llvm_profile_set_filename(),
> Index: lib/profile/InstrProfilingBuffer.c
> ===================================================================
> --- lib/profile/InstrProfilingBuffer.c
> +++ lib/profile/InstrProfilingBuffer.c
> @@ -68,11 +68,14 @@
>     * Match logic in __llvm_profile_write_file().
>     */
>
> +  __llvm_profile_value_data *ValueDataBegin = NULL;
> +
>    /* Calculate size of sections. */
>    const uint64_t DataSize = DataEnd - DataBegin;
>    const uint64_t CountersSize = CountersEnd - CountersBegin;
>    const uint64_t NamesSize = NamesEnd - NamesBegin;
>    const uint64_t Padding = sizeof(uint64_t) - NamesSize % sizeof(uint64_t);
> +  const uint64_t ValueDataSize = 0;

I guess this is supposed to do:

  __llvm_profile_gather_value_data(&ValueDataBegin);

?

>
>    /* Enough zeroes for padding. */
>    const char Zeroes[sizeof(uint64_t)] = {0};
> @@ -86,6 +89,8 @@
>    Header[4] = NamesSize;
>    Header[5] = (uintptr_t)CountersBegin;
>    Header[6] = (uintptr_t)NamesBegin;
> +  Header[7] = ValueDataSize;
> +  Header[8] = (uintptr_t)ValueDataBegin;
>
>    /* Write the data. */
>  #define UPDATE_memcpy(Data, Size) \
> Index: lib/profile/InstrProfilingFile.c
> ===================================================================
> --- lib/profile/InstrProfilingFile.c
> +++ lib/profile/InstrProfilingFile.c
> @@ -23,12 +23,20 @@
>    const uint64_t *CountersEnd   = __llvm_profile_end_counters();
>    const char *NamesBegin = __llvm_profile_begin_names();
>    const char *NamesEnd   = __llvm_profile_end_names();
> +  __llvm_profile_value_data *ValueDataBegin = NULL;
>
>    /* Calculate size of sections. */
>    const uint64_t DataSize = DataEnd - DataBegin;
>    const uint64_t CountersSize = CountersEnd - CountersBegin;
>    const uint64_t NamesSize = NamesEnd - NamesBegin;
>    const uint64_t Padding = sizeof(uint64_t) - NamesSize % sizeof(uint64_t);
> +  const uint64_t ValueDataSize =
> +    __llvm_profile_gather_value_data(&ValueDataBegin);
> +
> +  const uint64_t ValueDataSizeInBytes =
> +    ValueDataSize * sizeof(__llvm_profile_value_data);
> +  const uint64_t Padding2 = sizeof(uint64_t) -
> +    ValueDataSizeInBytes % sizeof(uint64_t);
>
>    /* Enough zeroes for padding. */
>    const char Zeroes[sizeof(uint64_t)] = {0};
> @@ -42,6 +50,8 @@
>    Header[4] = NamesSize;
>    Header[5] = (uintptr_t)CountersBegin;
>    Header[6] = (uintptr_t)NamesBegin;
> +  Header[7] = ValueDataSize;
> +  Header[8] = (uintptr_t)ValueDataBegin;
>
>    /* Write the data. */
>  #define CHECK_fwrite(Data, Size, Length, File) \
> @@ -51,8 +61,11 @@
>    CHECK_fwrite(CountersBegin, sizeof(uint64_t), CountersSize, File);
>    CHECK_fwrite(NamesBegin,    sizeof(char), NamesSize, File);
>    CHECK_fwrite(Zeroes,        sizeof(char), Padding, File);
> +  CHECK_fwrite(ValueDataBegin,
> +    sizeof(__llvm_profile_value_data), ValueDataSize, File);
> +  CHECK_fwrite(Zeroes,        sizeof(char), Padding2, File);
>  #undef CHECK_fwrite
> -
> +  free(ValueDataBegin);
>    return 0;
>  }
>



More information about the llvm-commits mailing list