[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