[PGO] Refactor common profile dumping code for File and Buffer API

[davidxl]

Summary

The implementation of the buffer API and File API share almost identical code causing lots of duplicate update work. This patch refactors the implementation using a writer callback mechanism.

[vsk]

Thanks, just a few nits --

lib/profile/InstrProfilingBuffer.c

44 ↗

(On Diff #40249)

ElmS -> ElmSize?

44 ↗

(On Diff #40249)

C code in compiler-rt looks like it uses all lowercase and "_" separators in function names. We should preserve that style.

lib/profile/InstrProfilingFile.c

20 ↗

(On Diff #40249)

Same ElmS nit

lib/profile/InstrProfilingWriter.c

57 ↗

(On Diff #40249)

CHECK_fwrite -> CHECK_write

[davidxl]

lib/profile/InstrProfilingBuffer.c

44 ↗

(On Diff #40249)

yes public C APIs are all lower cases, but these two new functions are internal APIs (just like existing functions like writeFile etc). They have global linkage mainly because the cross module references.

[davidxl]

Updated patch according to Vedant's comments.

[vsk]

Lgtm.

lib/profile/InstrProfilingBuffer.c

44–50 ↗

(On Diff #40311)

Ah ok

compiler-rt/trunk/lib/profile/CMakeLists.txt

 add_custom_target(profile)
 
 set(PROFILE_SOURCES
   GCDAProfiling.c
   InstrProfiling.c
   InstrProfilingBuffer.c
   InstrProfilingFile.c
+  InstrProfilingWriter.c
   InstrProfilingPlatformDarwin.c
   InstrProfilingPlatformLinux.c
   InstrProfilingPlatformOther.c
   InstrProfilingRuntime.cc
   InstrProfilingUtil.c)
 
 if(UNIX)
  set(EXTRA_FLAGS

compiler-rt/trunk/lib/profile/InstrProfilingBuffer.c

uint64_t __llvm_profile_get_size_for_buffer_internal(
         const __llvm_profile_data *DataBegin,
         const __llvm_profile_data *DataEnd, const uint64_t *CountersBegin,
         const uint64_t *CountersEnd, const char *NamesBegin,
         const char *NamesEnd) {
   /* Match logic in __llvm_profile_write_buffer(). */
   const uint64_t NamesSize = PROFILE_RANGE_SIZE(Names) * sizeof(char);
   const uint8_t Padding = __llvm_profile_get_num_padding_bytes(NamesSize);
   return sizeof(__llvm_profile_header) +
          PROFILE_RANGE_SIZE(Data) * sizeof(__llvm_profile_data) +
-      PROFILE_RANGE_SIZE(Counters) * sizeof(uint64_t) +
-      NamesSize + Padding;
+         PROFILE_RANGE_SIZE(Counters) * sizeof(uint64_t) + NamesSize + Padding;
 }
 
-__attribute__((visibility("hidden")))
-int __llvm_profile_write_buffer(char *Buffer) {
-  /* Match logic in __llvm_profile_get_size_for_buffer().
-   * Match logic in __llvm_profile_write_file().
-   */
-  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();
+static size_t BufferWriter(const void *Data, size_t ElmSize, size_t NumElm,
+                           void **Buffer) {
+  size_t Length = ElmSize * NumElm;
+  memcpy(*Buffer, Data, Length);
+  *(char **)Buffer += Length;
+  return NumElm;
+}
 
-  return __llvm_profile_write_buffer_internal(Buffer, DataBegin, DataEnd,
-                                              CountersBegin, CountersEnd,
-                                              NamesBegin, NamesEnd);
+__attribute__((visibility("hidden"))) int
+__llvm_profile_write_buffer(char *Buffer) {
+  return llvmWriteProfData(Buffer, 0, 0, BufferWriter);
 }
 
-__attribute__((visibility("hidden")))
-int __llvm_profile_write_buffer_internal(
+__attribute__((visibility("hidden"))) int __llvm_profile_write_buffer_internal(
     char *Buffer, const __llvm_profile_data *DataBegin,
     const __llvm_profile_data *DataEnd, const uint64_t *CountersBegin,
     const uint64_t *CountersEnd, const char *NamesBegin, const char *NamesEnd) {
-  /* Match logic in __llvm_profile_get_size_for_buffer().
-   * Match logic in __llvm_profile_write_file().
-   */
-
-  /* Calculate size of sections. */
-  const uint64_t DataSize = DataEnd - DataBegin;
-  const uint64_t CountersSize = CountersEnd - CountersBegin;
-  const uint64_t NamesSize = NamesEnd - NamesBegin;
-  const uint8_t Padding = __llvm_profile_get_num_padding_bytes(NamesSize);
-
-  /* Enough zeroes for padding. */
-  const char Zeroes[sizeof(uint64_t)] = {0};
-
-  /* Create the header. */
-  __llvm_profile_header Header;
-
-  if (!DataSize)
-    return 0;
-
-  Header.Magic = __llvm_profile_get_magic();
-  Header.Version = __llvm_profile_get_version();
-  Header.DataSize = DataSize;
-  Header.CountersSize = CountersSize;
-  Header.NamesSize = NamesSize;
-  Header.CountersDelta = (uintptr_t)CountersBegin;
-  Header.NamesDelta = (uintptr_t)NamesBegin;
-  Header.ValueKindLast = VK_LAST;
-  Header.ValueDataSize = 0;
-  Header.ValueDataDelta = (uintptr_t)NULL;
-
-  /* Write the data. */
-#define UPDATE_memcpy(Data, Size) \
-  do {                            \
-    memcpy(Buffer, Data, Size);   \
-    Buffer += Size;               \
-  } while (0)
-  UPDATE_memcpy(&Header,  sizeof(__llvm_profile_header));
-  UPDATE_memcpy(DataBegin,     DataSize      * sizeof(__llvm_profile_data));
-  UPDATE_memcpy(CountersBegin, CountersSize  * sizeof(uint64_t));
-  UPDATE_memcpy(NamesBegin,    NamesSize     * sizeof(char));
-  UPDATE_memcpy(Zeroes,        Padding       * sizeof(char));
-#undef UPDATE_memcpy
-
-  return 0;
+  return llvmWriteProfDataImpl(Buffer, BufferWriter, DataBegin, DataEnd,
+                               CountersBegin, CountersEnd, 0, 0, NamesBegin,
+                               NamesEnd);
 }

compiler-rt/trunk/lib/profile/InstrProfilingFile.c

 /*===- InstrProfilingFile.c - Write instrumentation to a file -------------===*\
 |*
 |*                     The LLVM Compiler Infrastructure
 |*
 |* This file is distributed under the University of Illinois Open Source
 |* License. See LICENSE.TXT for details.
 |*
 \*===----------------------------------------------------------------------===*/
 
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
+#include "InstrProfilingInternal.h"
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 #define UNCONST(ptr) ((void *)(uintptr_t)(ptr))
 
-static int writeFile(FILE *File) {
-  /* 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();
+static size_t FileWriter(const void *Data, size_t ElmSize, size_t NumElm,
+                         void **File) {
+  return fwrite(Data, ElmSize, NumElm, (FILE *)*File);
+}
   uint8_t *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 uint8_t Padding = __llvm_profile_get_num_padding_bytes(NamesSize);
-  const uint64_t ValueDataSize =
-      __llvm_profile_gather_value_data(&ValueDataBegin);
-
-  /* Enough zeroes for padding. */
-  const char Zeroes[sizeof(uint64_t)] = {0};
-
-  /* Create the header. */
-  __llvm_profile_header Header;
-
-  if (!DataSize)
-    return 0;
-
-  Header.Magic = __llvm_profile_get_magic();
-  Header.Version = __llvm_profile_get_version();
-  Header.DataSize = DataSize;
-  Header.CountersSize = CountersSize;
-  Header.NamesSize = NamesSize;
-  Header.CountersDelta = (uintptr_t)CountersBegin;
-  Header.NamesDelta = (uintptr_t)NamesBegin;
-  Header.ValueKindLast = VK_LAST;
-  Header.ValueDataSize = ValueDataSize;
-  Header.ValueDataDelta = (uintptr_t)ValueDataBegin;
-
-  /* Write the data. */
-#define CHECK_fwrite(Data, Size, Length, File) \
-  do { if (fwrite(Data, Size, Length, File) != Length) return -1; } while (0)
-  CHECK_fwrite(&Header,       sizeof(__llvm_profile_header), 1, File);
-  CHECK_fwrite(DataBegin,     sizeof(__llvm_profile_data), DataSize, File);
-  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);
-#undef CHECK_fwrite
-  free(ValueDataBegin);
-  return 0;
+static int writeFile(FILE *File) {
+  uint8_t *ValueDataBegin = NULL;
+  const uint64_t ValueDataSize = __llvm_profile_gather_value_data(&ValueDataBegin);
+  int r = llvmWriteProfData(File, ValueDataBegin, ValueDataSize, FileWriter);
+  free (ValueDataBegin);
+  return r;
 }
 
 static int writeFileWithName(const char *OutputName) {
   int RetVal;
   FILE *OutputFile;
   if (!OutputName || !OutputName[0])
     return -1;
 

compiler-rt/trunk/lib/profile/InstrProfilingInternal.h

 /*===- InstrProfiling.h- Support library for PGO instrumentation ----------===*\
 |*
 |*                     The LLVM Compiler Infrastructure
 |*
 |* This file is distributed under the University of Illinois Open Source
 |* License. See LICENSE.TXT for details.
 |*
 \*===----------------------------------------------------------------------===*/
 
 #ifndef PROFILE_INSTRPROFILING_INTERNALH_
 #define PROFILE_INSTRPROFILING_INTERNALH_
 
 #include "InstrProfiling.h"
+#include "stddef.h"
 
 /*!
  * \brief Write instrumentation data to the given buffer, given explicit
  * pointers to the live data in memory.  This function is probably not what you
  * want.  Use __llvm_profile_get_size_for_buffer instead.  Use this function if
  * your program has a custom memory layout.
  */
 uint64_t __llvm_profile_get_size_for_buffer_internal(

  * \pre \c Buffer is the start of a buffer at least as big as \a
  * __llvm_profile_get_size_for_buffer_internal().
  */
 int __llvm_profile_write_buffer_internal(
     char *Buffer, const __llvm_profile_data *DataBegin,
     const __llvm_profile_data *DataEnd, const uint64_t *CountersBegin,
     const uint64_t *CountersEnd, const char *NamesBegin, const char *NamesEnd);
 
+/*!
+ * This is an internal function not intended to be used externally.
+ */
+typedef size_t (*WriterCallback)(const void *Data, size_t ElmS, size_t NumElm,
+                                 void **BufferOrFile);
+int llvmWriteProfData(void *BufferOrFile, const uint8_t *ValueDataBegin, const uint64_t ValueDataSize, WriterCallback Writer);
+int llvmWriteProfDataImpl(void *BufferOrFile, WriterCallback Writer,
+                          const __llvm_profile_data *DataBegin,
+                          const __llvm_profile_data *DataEnd,
+                          const uint64_t *CountersBegin, const uint64_t *CountersEnd,
+                          const uint8_t *ValueDataBegin, const uint64_t ValueDataSize,
+                          const char *NamesBegin,
+                          const char *NamesEnd);
+
 #endif

compiler-rt/trunk/lib/profile/InstrProfilingWriter.c

+/*===- InstrProfilingWriter.c - Write instrumentation to a file or buffer -===*\
+|*
+|*                     The LLVM Compiler Infrastructure
+|*
+|* This file is distributed under the University of Illinois Open Source
+|* License. See LICENSE.TXT for details.
+|*
+\*===----------------------------------------------------------------------===*/
+
+#include "InstrProfiling.h"
+#include "InstrProfilingInternal.h"
+
+__attribute__((visibility("hidden"))) int
+llvmWriteProfData(void *BufferOrFile, const uint8_t * ValueDataBegin, const uint64_t ValueDataSize,  WriterCallback Writer) {
+  /* 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 llvmWriteProfDataImpl(BufferOrFile, Writer, DataBegin, DataEnd,
+                               CountersBegin, CountersEnd,
+                               ValueDataBegin, ValueDataSize,
+                               NamesBegin,
+                               NamesEnd);
+}
+
+__attribute__((visibility("hidden"))) int llvmWriteProfDataImpl(
+    void *BufferOrFile, WriterCallback Writer,
+    const __llvm_profile_data *DataBegin, const __llvm_profile_data *DataEnd,
+    const uint64_t *CountersBegin, const uint64_t *CountersEnd,
+    const uint8_t *ValueDataBegin, const uint64_t ValueDataSize,
+    const char *NamesBegin, const char *NamesEnd) {
+
+  /* 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 = __llvm_profile_get_num_padding_bytes(NamesSize);
+
+  /* Enough zeroes for padding. */
+  const char Zeroes[sizeof(uint64_t)] = {0};
+
+  /* Create the header. */
+  __llvm_profile_header Header;
+
+  if (!DataSize)
+    return 0;
+
+  Header.Magic = __llvm_profile_get_magic();
+  Header.Version = __llvm_profile_get_version();
+  Header.DataSize = DataSize;
+  Header.CountersSize = CountersSize;
+  Header.NamesSize = NamesSize;
+  Header.CountersDelta = (uintptr_t)CountersBegin;
+  Header.NamesDelta = (uintptr_t)NamesBegin;
+  Header.ValueKindLast = VK_LAST;
+  Header.ValueDataSize = ValueDataSize;
+  Header.ValueDataDelta = (uintptr_t)ValueDataBegin;
+
+/* Write the data. */
+#define CHECK_write(Data, Size, Length, BuffOrFile)                           \
+  do {                                                                        \
+    if (Writer(Data, Size, Length, &BuffOrFile) != Length)                    \
+      return -1;                                                              \
+  } while (0)
+  CHECK_write(&Header, sizeof(__llvm_profile_header), 1, BufferOrFile);
+  CHECK_write(DataBegin, sizeof(__llvm_profile_data), DataSize, BufferOrFile);
+  CHECK_write(CountersBegin, sizeof(uint64_t), CountersSize, BufferOrFile);
+  CHECK_write(NamesBegin, sizeof(char), NamesSize, BufferOrFile);
+  CHECK_write(Zeroes, sizeof(char), Padding, BufferOrFile);
+  if (ValueDataBegin)
+    CHECK_write(ValueDataBegin, sizeof(char), ValueDataSize, BufferOrFile);
+#undef CHECK_write
+  return 0;
+}