[PATCH] D17831: [PGO] Add API to merge profile from buffer

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 2 15:24:29 PST 2016


vsk added a comment.

Lgtm with minor changes.


================
Comment at: lib/profile/InstrProfiling.h:66
@@ -65,1 +65,3 @@
 /*!
+ * \brief Read profile data form buffer and merge with
+ * in-process profile counters. The client is expected to
----------------
nit, from

================
Comment at: lib/profile/InstrProfilingMerge.c:27
@@ +26,3 @@
+      (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header));
+  SrcDataEnd = SrcDataStart + Header->DataSize;
+  SrcCountersStart = (uint64_t *)SrcDataEnd;
----------------
It's documented that the source/destination profile buffers should have the same layout, but we should return early if `data_size(data_begin(), data_end()) != Header->DataSize`. The same goes for `CountersSize` and `NamesSize`.

================
Comment at: lib/profile/InstrProfilingMerge.c:60
@@ +59,3 @@
+    VR = getFirstValueProfRecord(SrcValueProfData);
+    for (I = 0; I < SrcValueProfData->NumValueKinds; I++) {
+      VData = getValueProfRecordValueData(VR);
----------------
Please split this loop nest into a separate function.

================
Comment at: make/platform/clang_linux.mk:83
@@ -83,1 +82,3 @@
+                          InstrProfilingWriter InstrProfilingValue \
+			  InstrProfilingMerge
 FUNCTIONS.profile-x86_64 := $(FUNCTIONS.profile-i386)
----------------
nit, indentation


http://reviews.llvm.org/D17831





More information about the llvm-commits mailing list