[PATCH] D15057: [PGO] Enable common VP format in profile runtime

[Vedant Kumar via llvm-commits]

vsk added a subscriber: vsk.

================
Comment at: lib/profile/InstrProfiling.c:66
@@ -62,6 +65,3 @@
 
-/* Total number of value profile data in bytes. */
-static uint64_t TotalValueDataSize = 0;
-
-#ifdef _MIPS_ARCH
+/* This methid is only used in value profiler mock testing.  */
 LLVM_LIBRARY_VISIBILITY void
----------------
nit, method

================
Comment at: lib/profile/InstrProfiling.c:117
@@ +116,3 @@
+    }
+  }
+}
----------------
Is `Data->Values` supposed to be free'd at some point?

================
Comment at: lib/profile/InstrProfiling.c:175
@@ +174,3 @@
+  __llvm_profile_data *I;
+  ValueProfRuntimeRecord *Records = 0;
+  if (!VDataArray)
----------------
`NULL`?

================
Comment at: test/profile/instrprof-value-prof.c:4
@@ +3,3 @@
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-profdata show --all-functions -ic-targets  %t.profdata | FileCheck
+// %s
----------------
I might be doing this incorrectly but I get a syntax error here. Doesn't this need to be:

    // RUN: llvm-profdata show --all-functions -ic-targets  %t.profdata | FileCheck \
    // RUN: %s

================
Comment at: test/profile/instrprof-value-prof.c:51
@@ +50,3 @@
+  void *addr2 = *(void **)p2;
+  if (addr1 < addr2)
+    return 1;
----------------
`return (intptr_t)p2 - (intptr_t)p1`?

================
Comment at: test/profile/instrprof-value-prof.c:95
@@ +94,3 @@
+// CHECK-NEXT: Indirect Target Results:
+// CHECK-NEXT:  [ 1, callee_2_2_2, 1 ]
+// CHECK-NEXT:  [ 2, callee_2_2_2, 1 ]
----------------
Will the CHECK lines break if the assumed mapping between function names and addresses changes?


http://reviews.llvm.org/D15057