[PATCH] Indirect call target profiling compiler-rt changes

David davidxl at google.com
Tue May 5 13:12:28 PDT 2015


================
Comment at: lib/profile/InstrProfiling.c:49
@@ -47,1 +48,3 @@
   memset(I, 0, sizeof(uint64_t)*(E - I));
+
+  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
----------------
Add an one liner comment here -- or refactor the following code into a different method to make the intention clear.

================
Comment at: lib/profile/InstrProfiling.c:88
@@ +87,3 @@
+      calloc(Data->NumValueSites[ValueKind], sizeof(__llvm_profile_value_data*));
+    if (!Mem)
+      return;
----------------
Out of memory condition is severe.  At least a warning is needed here.

================
Comment at: lib/profile/InstrProfiling.c:112
@@ +111,3 @@
+    calloc(1, sizeof(__llvm_profile_value_data));
+  if (!CurrentValueData)
+    return;
----------------
Not good to silently drop error here.

================
Comment at: lib/profile/InstrProfiling.c:139
@@ +138,3 @@
+
+  if (!ValueProfilingResults)
+    return 0;
----------------
Assert or just return?

================
Comment at: lib/profile/InstrProfiling.c:146
@@ +145,3 @@
+  uint64_t NumDataCount = TotalValueDataCount;
+  *ValueProfilingResults = (__llvm_profile_value_data*)
+    calloc(NumDataCount, sizeof(__llvm_profile_value_data));
----------------
do null return check first before assignment. Report error.

================
Comment at: lib/profile/InstrProfiling.h:82
@@ +81,3 @@
+ * Records the target value for a given ValueKind if not seen before. Otherwise,
+ * increments the counter associated w/ the target value.
+ */
----------------
Putting more description of the parameters in the documentation.

================
Comment at: lib/profile/InstrProfiling.h:85
@@ +84,3 @@
+void __llvm_profile_instrument_target(uint32_t ValueKind,
+  uint64_t TargetValue, void *Data_, uint32_t CounterIndex);
+
----------------
why Data_ instead of Data?

================
Comment at: lib/profile/InstrProfiling.h:88
@@ +87,3 @@
+/*!
+ * \brief Prepares the value profiling data for output.
+ *
----------------
The description of this interface is too sparse. How about adding for instance:
1) when it is expected to be used
2) the input and output. 
3) ownership of the allocation etc.

http://reviews.llvm.org/D9009

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list