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

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Tue May 10 15:18:42 PDT 2016


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/5f641716/attachment.html>


More information about the llvm-commits mailing list