[Profile] introduce interface __llvm_profile_dump

[davidxl]

Summary

This new interface is intended to be used to collect profile in hand selected hot regions. It is a wrapper to __llvm_profile_write_file. The behavior of the latter (existing one) does not change.

[vsk]

Neat! The test looks good.

lib/profile/InstrProfiling.c

81

Should probably also be cleared when llvm_prof_set_filename() or llvm_profile_merge_from_buffer() are called.

[silvas]

lib/profile/InstrProfiling.h

135

I don't think it is clear to the user what "on-line profile merging is on" means. AFAIK, currently that implicitly gets turned on based on the filename.
Maybe it would be clearer to say that "the filename must contain a %m specifier"? (possibly in addition to what you already have).

[silvas]

(other than that, this LGTM).

[davidxl]

lib/profile/InstrProfiling.c

81

It is a mistake to not call reset counter before dumping again (due to double counting) regardless whether merging is used or not

[davidxl]

Updated comments according to review feedback

[vsk]

lib/profile/InstrProfiling.c

81

Oh, I see. This sequence of calls doesn't make sense: dump(), merge_profile_from_buffer(), dump(). You're saying you'd need to reset the counters some time before the second call to dump() anyway.

What about: dump(), set_filename(), dump()?. The second 'dump()' wouldn't go through, which seems like a problem.

lib/profile/InstrProfiling.h

  * __llvm_profile_set_filename(),
  * or if it hasn't been called, the \c LLVM_PROFILE_FILE environment variable,
  * or if that's not set, the last name set to INSTR_PROF_PROFILE_NAME_VAR,
  * or if that's not set,  \c "default.profraw".
  */
 int __llvm_profile_write_file(void);
 
 /*!
+ * \brief this is a wrapper interface to \c __llvm_profile_write_file.
+ * After this interface is invoked, a arleady dumped flag will be set
+ * so that profile won't be dumped again during program exit. 
+ * Invocation of interface __llvm_profile_reset_counters will clear
+ * the flag. This interface is designed to be used to collect profile
+ * data from user selected hot regions. The use model is
+ *      __llvm_profile_reset_counters();
+ *      ... hot region 1
+ *      __llvm_profile_dump();
+ *      .. some other code
+ *      __llvm_profile_reset_counters();
+ *       ... hot region 2
+ *      __llvm_profile_dump();
+ *
+ *  It is expected that on-line profile merging is on. If merging is
+ *  not turned on, user is expected to invoke __llvm_profile_set_filename
+ *  to specify different profile names for different regions before
+ *  dumping to avoid profile write clobbering.
+ */
+int __llvm_profile_dump(void);
+
+/*!
  * \brief Set the filename for writing instrumentation data.
  *
  * Sets the filename to be used for subsequent calls to
  * \a __llvm_profile_write_file().
  *
  * \c Name is not copied, so it must remain valid.  Passing NULL resets the
  * filename logic to the default behaviour.
  */

lib/profile/InstrProfiling.c

 
 COMPILER_RT_WEAK char INSTR_PROF_PROFILE_NAME_VAR[1] = {0};
 
 COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_magic(void) {
   return sizeof(void *) == sizeof(uint64_t) ? (INSTR_PROF_RAW_MAGIC_64)
                                             : (INSTR_PROF_RAW_MAGIC_32);
 }
 
+static unsigned ProfileDumped = 0;
+
+COMPILER_RT_VISIBILITY unsigned lprofProfileDumped() {
+  return ProfileDumped;
+}
+
+COMPILER_RT_VISIBILITY void lprofSetProfileDumped() {
+  ProfileDumped = 1;
+}
+
 /* Return the number of bytes needed to add to SizeInBytes to make it
  *   the result a multiple of 8.
  */
 COMPILER_RT_VISIBILITY uint8_t
 __llvm_profile_get_num_padding_bytes(uint64_t SizeInBytes) {
   return 7 & (sizeof(uint64_t) - SizeInBytes % sizeof(uint64_t));
 }
 
for (i = 0; i < CurrentVSiteCount; ++i) {
       ValueProfNode *CurrentVNode = ValueCounters[i];
 
       while (CurrentVNode) {
         CurrentVNode->Count = 0;
         CurrentVNode = CurrentVNode->Next;
       }
     }
   }
+  ProfileDumped = 0;
 }

lib/profile/InstrProfilingFile.c

  * __llvm_profile_override_default_filename or
  * __llvm_profile_initialize_file. */
 COMPILER_RT_VISIBILITY
 int __llvm_profile_write_file(void) {
   int rc, Length;
   const char *Filename;
   char *FilenameBuf;
 
+  if (lprofProfileDumped()) {
+    PROF_NOTE("Profile data not written to file: %s.\n", 
+              "already written");
+    return 0;
+  }
+
   Length = getCurFilenameLength();
   FilenameBuf = (char *)COMPILER_RT_ALLOCA(Length + 1);
   Filename = getCurFilename(FilenameBuf);
 
   /* Check the filename. */
   if (!Filename) {
     PROF_ERR("Failed to write file : %s\n", "Filename not set");
     return -1;

 
   /* Write profile data to the file. */
   rc = writeFile(Filename);
   if (rc)
     PROF_ERR("Failed to write file \"%s\": %s\n", Filename, strerror(errno));
   return rc;
 }
 
+COMPILER_RT_VISIBILITY
+int __llvm_profile_dump(void) {
+  if (!doMerging())
+    PROF_WARN("Later invocation of __llvm_profile_dump can lead to clobbering "
+              " of previously dumped profile data : %s. Either use \%m "
+              "in profile name or change profile name before dumping.\n",
+              "online profile merging is not on");
+  int rc = __llvm_profile_write_file();
+  lprofSetProfileDumped();
+  return rc;
+}
+
 static void writeFileWithoutReturn(void) { __llvm_profile_write_file(); }
 
 COMPILER_RT_VISIBILITY
 int __llvm_profile_register_write_file_atexit(void) {
   static int HasBeenRegistered = 0;
 
   if (HasBeenRegistered)
     return 0;
 
   lprofSetupValueProfiler();
 
   HasBeenRegistered = 1;
   return atexit(writeFileWithoutReturn);
 }

lib/profile/InstrProfilingInternal.h

 
 /* Return the profile header 'signature' value associated with the current
  * executable or shared library. The signature value can be used to for
  * a profile name that is unique to this load module so that it does not
  * collide with profiles from other binaries. It also allows shared libraries
  * to dump merged profile data into its own profile file. */
 uint64_t lprofGetLoadModuleSignature();
 
+/* 
+ * Return non zero value if the profile data has already been
+ * dumped to the file.
+ */
+unsigned lprofProfileDumped();
+void lprofSetProfileDumped();
+
 COMPILER_RT_VISIBILITY extern char *(*GetEnvHook)(const char *);
 COMPILER_RT_VISIBILITY extern void (*FreeHook)(void *);
 COMPILER_RT_VISIBILITY extern uint8_t *DynamicBufferIOBuffer;
 COMPILER_RT_VISIBILITY extern uint32_t VPBufferSize;
 COMPILER_RT_VISIBILITY extern uint32_t VPMaxNumValsPerSite;
 /* Pointer to the start of static value counters to be allocted. */
 COMPILER_RT_VISIBILITY extern ValueProfNode *CurrentVNode;
 COMPILER_RT_VISIBILITY extern ValueProfNode *EndVNode;
 extern void (*VPMergeHook)(struct ValueProfData *, __llvm_profile_data *);
 
 #endif

test/profile/instrprof-dump.c

+/*
+RUN: rm -fr %t.profdir
+RUN: %clang_profgen=%t.profdir/default_%m.profraw -o %t -O2 %s
+RUN: %run %t  2>&1 | FileCheck %s --check-prefix=NO_EXIT_WRITE
+RUN: llvm-profdata merge -o %t.profdata %t.profdir
+RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s  --check-prefix=PROF
+
+NO_EXIT_WRITE: Profile data not written to file: already written
+*/
+
+int __llvm_profile_dump(void);
+void __llvm_profile_reset_counters(void);
+int foo(int);
+int bar(int);
+int skip(int);
+
+int main(int argc, const char *argv[]) {
+  int Ret = foo(0); /* region 1 */
+  __llvm_profile_dump();
+
+  /* not profiled -- cleared later. */
+  skip(0);   /* skipped region */
+  
+  __llvm_profile_reset_counters();
+  Ret += bar(0);  /* region 2 */
+  __llvm_profile_dump();
+
+  skip(1);
+
+  __llvm_profile_reset_counters();
+  /* foo's profile will be merged.  */
+  foo(1);  /* region 3 */
+  __llvm_profile_dump();
+
+  return Ret;
+}
+
+__attribute__((noinline)) int foo(int X) {
+  /* PROF: define {{.*}} @foo({{.*}}!prof ![[ENT:[0-9]+]]
+     PROF: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
+  */
+  return X <= 0 ? -X : X;
+}
+
+__attribute__((noinline)) int skip(int X) {
+  /* PROF: define {{.*}} @skip(
+     PROF: br i1 %{{.*}}, label %{{.*}}, label %{{[^,]+$}}
+  */
+  return X <= 0 ? -X : X;
+}
+
+__attribute__((noinline)) int bar(int X) {
+  /* PROF-LABEL: define {{.*}} @bar(
+     PROF: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD2:[0-9]+]]
+  */
+  return X <= 0 ? -X : X;
+}
+
+/*
+PROF: ![[ENT]] = !{!"function_entry_count", i64 2}  
+PROF: ![[PD1]] = !{!"branch_weights", i32 2, i32 2}
+*/