[PATCH] D9009: Value profiling compiler-rt changes

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/profile/InstrProfiling.c:81
@@ +80,3 @@
+void __llvm_profile_instrument_target(uint64_t TargetValue, void *Data_,
+  uint32_t CounterIndex) {
+
----------------
Can we make the CounterIndex relative to the value kind? Why is value kind parameter not passed in?

================
Comment at: lib/profile/InstrProfiling.c:87
@@ +86,3 @@
+
+  if (!Data->ValueCounters) {
+    uint64_t NumVSites = 0;
----------------
This part of the code is cold -- move it out of line into another routine.

================
Comment at: lib/profile/InstrProfiling.c:119
@@ +118,3 @@
+
+  if (VDataCount >= UCHAR_MAX)
+    return;
----------------
This may still be too large. If VDataCount is this large, the runtime overhead will be substantial. 

In the future, we should enhance it with the following algoithm:
1) reserve the first vdata node to do book keeping -- i.e. tracking number of time values get dropped due to too long a list
2) When the number of dropping times reaches a threshold, sort the existing linked list to make it in descending order, and zero out the last X entries.

Not needed for the first version.

================
Comment at: lib/profile/InstrProfiling.h:33
@@ +32,3 @@
+  VK_FIRST = VK_IndirectCallTarget,
+  VK_LAST = VK_IndirectCallTarget
+} __llvm_profile_value_kind;
----------------
Make the enumerators name the same as the LLVM counter part -- this allows future refactoring.


http://reviews.llvm.org/D9009