[compiler-rt] r268992 - Reapply r268840: [profile] Simplify value profile writing

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Tue May 10 15:42:58 PDT 2016


segfaults in tests or real programs?

Before this patch, the value profile gatherer also allocates memory, i
suppose you have a local patch to shortcut it?


On the other hand, I am also thinking getting rid of dynamic allocation in
VP completely, but instead using a pool of statiically allocated nodes
instead -- but this requires some heuritic to guess the suitable size of
the pool during compile time.

thanks,

David

On Tue, May 10, 2016 at 3:18 PM, Sean Silva <chisophugis at gmail.com> wrote:

> Hi David,
>
> On PS4 we were relying on graceful handling of the case where calloc
> returns NULL, and this patch breaks that I think (we're seeing segfaults).
> Is there a way that we can take a "shortcut" and avoid doing any memory
> allocation when there is no value profiling data to write out?
>
> On PS4, games typically use all available memory so value profiling (at
> least in its current form which calls into malloc) is not viable,
> unfortunately. In our testing, we usually disable VP (we will need to find
> a way in clang to control this in the driver based on the triple, but that
> is a discussion for another day).
>
> -- Sean Silva
>
> On Mon, May 9, 2016 at 5:17 PM, Xinliang David Li via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: davidxl
>> Date: Mon May  9 19:17:31 2016
>> New Revision: 268992
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=268992&view=rev
>> Log:
>> Reapply r268840: [profile] Simplify value profile writing
>>
>> Revert r268864 that reverted 268840 after underlying problem
>> is fixed for arm bot.
>>
>>
>>
>> Modified:
>>     compiler-rt/trunk/lib/profile/InstrProfiling.h
>>     compiler-rt/trunk/lib/profile/InstrProfilingBuffer.c
>>     compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>>     compiler-rt/trunk/lib/profile/InstrProfilingInternal.h
>>     compiler-rt/trunk/lib/profile/InstrProfilingValue.c
>>     compiler-rt/trunk/lib/profile/InstrProfilingWriter.c
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfiling.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfiling.h?rev=268992&r1=268991&r2=268992&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfiling.h (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfiling.h Mon May  9 19:17:31
>> 2016
>> @@ -94,15 +94,7 @@ int __llvm_profile_check_compatibility(c
>>  void INSTR_PROF_VALUE_PROF_FUNC(
>>  #define VALUE_PROF_FUNC_PARAM(ArgType, ArgName, ArgLLVMType) ArgType
>> ArgName
>>  #include "InstrProfData.inc"
>> -);
>> -
>> -/*!
>> - * \brief Prepares the value profiling data for output.
>> - *
>> - * Returns an array of pointers to value profile data.
>> - */
>> -struct ValueProfData;
>> -struct ValueProfData **__llvm_profile_gather_value_data(uint64_t *Size);
>> +    );
>>
>>  /*!
>>   * \brief Write instrumentation data to the current file.
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingBuffer.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingBuffer.c?rev=268992&r1=268991&r2=268992&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingBuffer.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingBuffer.c Mon May  9
>> 19:17:31 2016
>> @@ -46,7 +46,7 @@ uint64_t __llvm_profile_get_size_for_buf
>>  }
>>
>>  COMPILER_RT_VISIBILITY int __llvm_profile_write_buffer(char *Buffer) {
>> -  return lprofWriteData(lprofBufferWriter, Buffer, 0, 0);
>> +  return lprofWriteData(lprofBufferWriter, Buffer, 0);
>>  }
>>
>>  COMPILER_RT_VISIBILITY int __llvm_profile_write_buffer_internal(
>> @@ -54,6 +54,6 @@ COMPILER_RT_VISIBILITY int __llvm_profil
>>      const __llvm_profile_data *DataEnd, const uint64_t *CountersBegin,
>>      const uint64_t *CountersEnd, const char *NamesBegin, const char
>> *NamesEnd) {
>>    return lprofWriteDataImpl(lprofBufferWriter, Buffer, DataBegin,
>> DataEnd,
>> -                            CountersBegin, CountersEnd, 0, 0, NamesBegin,
>> +                            CountersBegin, CountersEnd, 0, NamesBegin,
>>                              NamesEnd);
>>  }
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingFile.c?rev=268992&r1=268991&r2=268992&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingFile.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingFile.c Mon May  9
>> 19:17:31 2016
>> @@ -39,15 +39,12 @@ lprofCreateBufferIOInternal(void *File,
>>
>>  static int writeFile(FILE *File) {
>>    const char *BufferSzStr = 0;
>> -  uint64_t ValueDataSize = 0;
>> -  struct ValueProfData **ValueDataArray =
>> -      __llvm_profile_gather_value_data(&ValueDataSize);
>>    FreeHook = &free;
>>    CallocHook = &calloc;
>>    BufferSzStr = getenv("LLVM_VP_BUFFER_SIZE");
>>    if (BufferSzStr && BufferSzStr[0])
>>      VPBufferSize = atoi(BufferSzStr);
>> -  return lprofWriteData(fileWriter, File, ValueDataArray, ValueDataSize);
>> +  return lprofWriteData(fileWriter, File, lprofGatherValueProfData);
>>  }
>>
>>  static int writeFileWithName(const char *OutputName) {
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingInternal.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingInternal.h?rev=268992&r1=268991&r2=268992&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingInternal.h (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingInternal.h Mon May  9
>> 19:17:31 2016
>> @@ -98,17 +98,20 @@ int lprofBufferIOFlush(ProfBufferIO *Buf
>>  uint32_t lprofBufferWriter(ProfDataIOVec *IOVecs, uint32_t NumIOVecs,
>>                             void **WriterCtx);
>>
>> +typedef struct ValueProfData *(*VPGatherHookType)(
>> +    const __llvm_profile_data *Data);
>>  int lprofWriteData(WriterCallback Writer, void *WriterCtx,
>> -                   struct ValueProfData **ValueDataArray,
>> -                   const uint64_t ValueDataSize);
>> +                   VPGatherHookType VPDataGatherer);
>>  int lprofWriteDataImpl(WriterCallback Writer, void *WriterCtx,
>>                         const __llvm_profile_data *DataBegin,
>>                         const __llvm_profile_data *DataEnd,
>>                         const uint64_t *CountersBegin,
>>                         const uint64_t *CountersEnd,
>> -                       struct ValueProfData **ValueDataBeginArray,
>> -                       const uint64_t ValueDataSize, const char
>> *NamesBegin,
>> +                       VPGatherHookType VPDataGatherer, const char
>> *NamesBegin,
>>                         const char *NamesEnd);
>> +/* Gather value profile data from \c Data and return it. */
>> +struct ValueProfData *lprofGatherValueProfData(const __llvm_profile_data
>> *Data);
>> +
>>  /* Merge value profile data pointed to by SrcValueProfData into
>>   * in-memory profile counters pointed by to DstData.  */
>>  void lprofMergeValueProfData(struct ValueProfData *SrcValueProfData,
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingValue.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingValue.c?rev=268992&r1=268991&r2=268992&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingValue.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingValue.c Mon May  9
>> 19:17:31 2016
>> @@ -21,7 +21,6 @@
>>  #define PROF_OOM_RETURN(Msg)
>>        \
>>    {
>>       \
>>      PROF_OOM(Msg)
>>       \
>> -    free(ValueDataArray);
>>       \
>>      return NULL;
>>        \
>>    }
>>
>> @@ -118,51 +117,22 @@ __llvm_profile_instrument_target(uint64_
>>    }
>>  }
>>
>> -COMPILER_RT_VISIBILITY ValueProfData **
>> -__llvm_profile_gather_value_data(uint64_t *ValueDataSize) {
>> -  size_t S = 0;
>> -  __llvm_profile_data *I;
>> -  ValueProfData **ValueDataArray;
>> -
>> -  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
>> -  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
>> -
>> -  if (!ValueDataSize)
>> -    return NULL;
>> -
>> -  ValueDataArray = (ValueProfData **)calloc(
>> -      __llvm_profile_get_data_size(DataBegin, DataEnd), sizeof(void *));
>> -  if (!ValueDataArray)
>> +COMPILER_RT_VISIBILITY struct ValueProfData *
>> +lprofGatherValueProfData(const __llvm_profile_data *Data) {
>> +  ValueProfData *VD = NULL;
>> +  ValueProfRuntimeRecord R;
>> +  if (initializeValueProfRuntimeRecord(&R, Data->NumValueSites,
>> Data->Values))
>>      PROF_OOM_RETURN("Failed to write value profile data ");
>>
>> -  /*
>> -   * Compute the total Size of the buffer to hold ValueProfData
>> -   * structures for functions with value profile data.
>> -   */
>> -  for (I = (__llvm_profile_data *)DataBegin; I < DataEnd; ++I) {
>> -    ValueProfRuntimeRecord R;
>> -    if (initializeValueProfRuntimeRecord(&R, I->NumValueSites,
>> I->Values))
>> +  /* Compute the size of ValueProfData from this runtime record.  */
>> +  if (getNumValueKindsRT(&R) != 0) {
>> +    uint32_t VS = getValueProfDataSizeRT(&R);
>> +    VD = (ValueProfData *)calloc(VS, sizeof(uint8_t));
>> +    if (!VD)
>>        PROF_OOM_RETURN("Failed to write value profile data ");
>> -
>> -    /* Compute the size of ValueProfData from this runtime record.  */
>> -    if (getNumValueKindsRT(&R) != 0) {
>> -      ValueProfData *VD = NULL;
>> -      uint32_t VS = getValueProfDataSizeRT(&R);
>> -      VD = (ValueProfData *)calloc(VS, sizeof(uint8_t));
>> -      if (!VD)
>> -        PROF_OOM_RETURN("Failed to write value profile data ");
>> -      serializeValueProfDataFromRT(&R, VD);
>> -      ValueDataArray[I - DataBegin] = VD;
>> -      S += VS;
>> -    }
>> -    finalizeValueProfRuntimeRecord(&R);
>> -  }
>> -
>> -  if (!S) {
>> -    free(ValueDataArray);
>> -    ValueDataArray = NULL;
>> +    serializeValueProfDataFromRT(&R, VD);
>>    }
>> +  finalizeValueProfRuntimeRecord(&R);
>>
>> -  *ValueDataSize = S;
>> -  return ValueDataArray;
>> +  return VD;
>>  }
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingWriter.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingWriter.c?rev=268992&r1=268991&r2=268992&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingWriter.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingWriter.c Mon May  9
>> 19:17:31 2016
>> @@ -91,42 +91,29 @@ COMPILER_RT_VISIBILITY int lprofBufferIO
>>    return 0;
>>  }
>>
>> -COMPILER_RT_VISIBILITY int lprofWriteData(WriterCallback Writer,
>> -                                          void *WriterCtx,
>> -                                          ValueProfData **ValueDataArray,
>> -                                          const uint64_t ValueDataSize) {
>> -  /* Match logic in __llvm_profile_write_buffer(). */
>> -  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
>> -  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
>> -  const uint64_t *CountersBegin = __llvm_profile_begin_counters();
>> -  const uint64_t *CountersEnd = __llvm_profile_end_counters();
>> -  const char *NamesBegin = __llvm_profile_begin_names();
>> -  const char *NamesEnd = __llvm_profile_end_names();
>> -  return lprofWriteDataImpl(Writer, WriterCtx, DataBegin, DataEnd,
>> -                            CountersBegin, CountersEnd, ValueDataArray,
>> -                            ValueDataSize, NamesBegin, NamesEnd);
>> -}
>> -
>>  #define VP_BUFFER_SIZE 8 * 1024
>>  static int writeValueProfData(WriterCallback Writer, void *WriterCtx,
>> -                              ValueProfData **ValueDataBegin,
>> -                              uint64_t NumVData) {
>> +                              VPGatherHookType VPDataGatherer,
>> +                              const __llvm_profile_data *DataBegin,
>> +                              const __llvm_profile_data *DataEnd) {
>>    ProfBufferIO *BufferIO;
>> -  uint32_t I = 0, BufferSz;
>> +  uint32_t BufferSz;
>> +  const __llvm_profile_data *DI = 0;
>>
>> -  if (!ValueDataBegin)
>> +  if (!VPDataGatherer)
>>      return 0;
>>
>>    BufferSz = VPBufferSize ? VPBufferSize : VP_BUFFER_SIZE;
>>    BufferIO = lprofCreateBufferIO(Writer, WriterCtx, BufferSz);
>>
>> -  for (I = 0; I < NumVData; I++) {
>> -    ValueProfData *CurVData = ValueDataBegin[I];
>> +  for (DI = DataBegin; DI < DataEnd; DI++) {
>> +    ValueProfData *CurVData = VPDataGatherer(DI);
>>      if (!CurVData)
>>        continue;
>>      if (lprofBufferIOWrite(BufferIO, (const uint8_t *)CurVData,
>>                             CurVData->TotalSize) != 0)
>>        return -1;
>> +    FreeHook(CurVData);
>>    }
>>
>>    if (lprofBufferIOFlush(BufferIO) != 0)
>> @@ -136,13 +123,28 @@ static int writeValueProfData(WriterCall
>>    return 0;
>>  }
>>
>> +COMPILER_RT_VISIBILITY int lprofWriteData(WriterCallback Writer,
>> +                                          void *WriterCtx,
>> +                                          VPGatherHookType
>> VPDataGatherer) {
>> +  /* Match logic in __llvm_profile_write_buffer(). */
>> +  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
>> +  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
>> +  const uint64_t *CountersBegin = __llvm_profile_begin_counters();
>> +  const uint64_t *CountersEnd = __llvm_profile_end_counters();
>> +  const char *NamesBegin = __llvm_profile_begin_names();
>> +  const char *NamesEnd = __llvm_profile_end_names();
>> +  return lprofWriteDataImpl(Writer, WriterCtx, DataBegin, DataEnd,
>> +                            CountersBegin, CountersEnd, VPDataGatherer,
>> +                            NamesBegin, NamesEnd);
>> +}
>> +
>>  COMPILER_RT_VISIBILITY int
>>  lprofWriteDataImpl(WriterCallback Writer, void *WriterCtx,
>>                     const __llvm_profile_data *DataBegin,
>>                     const __llvm_profile_data *DataEnd,
>>                     const uint64_t *CountersBegin, const uint64_t
>> *CountersEnd,
>> -                   ValueProfData **ValueDataBegin, const uint64_t
>> ValueDataSize,
>> -                   const char *NamesBegin, const char *NamesEnd) {
>> +                   VPGatherHookType VPDataGatherer, const char
>> *NamesBegin,
>> +                   const char *NamesEnd) {
>>
>>    /* Calculate size of sections. */
>>    const uint64_t DataSize = __llvm_profile_get_data_size(DataBegin,
>> DataEnd);
>> @@ -159,7 +161,7 @@ lprofWriteDataImpl(WriterCallback Writer
>>    if (!DataSize)
>>      return 0;
>>
>> -  /* Initialize header struture.  */
>> +/* Initialize header structure.  */
>>  #define INSTR_PROF_RAW_HEADER(Type, Name, Init) Header.Name = Init;
>>  #include "InstrProfData.inc"
>>
>> @@ -172,5 +174,6 @@ lprofWriteDataImpl(WriterCallback Writer
>>    if (Writer(IOVec, sizeof(IOVec) / sizeof(*IOVec), &WriterCtx))
>>      return -1;
>>
>> -  return writeValueProfData(Writer, WriterCtx, ValueDataBegin, DataSize);
>> +  return writeValueProfData(Writer, WriterCtx, VPDataGatherer, DataBegin,
>> +                            DataEnd);
>>  }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160510/ad123f44/attachment.html>


More information about the llvm-commits mailing list