[PATCH] Indirect call target profiling compiler-rt changes

David davidxl at google.com
Wed Apr 29 10:17:38 PDT 2015


================
Comment at: lib/profile/InstrProfiling.c:72
@@ +71,3 @@
+static inline uint32_t __llvm_profile_mem_assign(uintptr_t *Dest, uintptr_t Src) {
+  static pthread_mutex_t DataMutex[64] = {PTHREAD_MUTEX_INITIALIZER};
+  uint32_t Success = 0;
----------------
No need for locking.

================
Comment at: lib/profile/InstrProfiling.c:75
@@ +74,3 @@
+  uint32_t Index = ((uintptr_t)Dest >> 3) & 0x3F;
+  pthread_mutex_lock(&DataMutex[Index]);
+  if (Dest && 0 == *Dest) {
----------------
No need for locking. The following cod can be replaced with

Success = __sync_bool_compare_and_swap(Dest, 0, Src);
return Success;

================
Comment at: lib/profile/InstrProfiling.c:85
@@ +84,3 @@
+__attribute__((visibility("hidden")))
+void __llvm_profile_instrument_indirect_call_site(uint8_t *TargetAddress,
+  void *Data_, uint32_t CounterIndex) {
----------------
Make the implementation more general matching the interface change.

================
Comment at: lib/profile/InstrProfiling.c:106
@@ +105,3 @@
+        (uintptr_t)Mem)) {
+      free(Mem);
+      return;
----------------
Consider defining and using allocator/deallocator interfaces (or macros).

================
Comment at: lib/profile/InstrProfiling.c:117
@@ +116,3 @@
+    if (TargetAddress == CurrentIndirectTargetData->TargetAddress) {
+      CurrentIndirectTargetData->NumTaken++;
+      return;
----------------
Consider using __sync_fetch_and_add

================
Comment at: lib/profile/InstrProfiling.c:175
@@ +174,3 @@
+
+    CurrentFunctionIndex++;
+    if (!I->IndirectCallSiteCounters)
----------------
Is there better function id to be used here? and how to detect mismatch?

================
Comment at: lib/profile/InstrProfiling.c:193
@@ +192,3 @@
+          (__llvm_profile_indirect_target_data*)CurrentFunctionIndex;
+        if (++CurrentResultIndex >= NumDataCount)
+          return NumDataCount;
----------------
How can this happen?

================
Comment at: lib/profile/InstrProfiling.h:34
@@ +33,3 @@
+
+typedef struct __llvm_profile_indirect_target_data {
+  const uint8_t *TargetAddress;
----------------
Perhaps change its name to __llvm_value_profile_target_data

================
Comment at: lib/profile/InstrProfiling.h:35
@@ +34,3 @@
+typedef struct __llvm_profile_indirect_target_data {
+  const uint8_t *TargetAddress;
+  uint64_t NumTaken;
----------------
change the type to uint64 and name to be TargetValue?

================
Comment at: lib/profile/InstrProfiling.h:39
@@ -31,1 +38,3 @@
+  struct __llvm_profile_indirect_target_data *Next;
+} __llvm_profile_indirect_target_data;
 
----------------
Change it ot __llvm_value_profile_target_data

================
Comment at: lib/profile/InstrProfiling.h:48
@@ +47,3 @@
+  const uint32_t NumIndirectCallSites;
+  const uint8_t *FunctionPointer;
+  __llvm_profile_indirect_target_data **IndirectCallSiteCounters;
----------------
__llvm_prf_names is one of the biggest contributor to the binary size bloat. In the longer run we will need to get rid of the name based profile matching and rely on profile_id (hash) plus source line and cfg cksum if needed.

When that happens, there will no need to record the function address because the value profiling site can be moved into the callee body which directly records the id associated with the callee.

For now, this is fine.

================
Comment at: lib/profile/InstrProfiling.h:78
@@ +77,3 @@
+ */
+void __llvm_profile_instrument_indirect_call_site(uint8_t *TargetAddress,
+  void *Data_, uint32_t CounterIndex);
----------------
Change the interface to be more general:

__llvm_instrument_value_profile 

Add one more parameter to indicate the value profile kind. For now, only IC promotion is defined

enum ValueProfileKind{
  IndirectCallTarget = 0,
 ....
};


================
Comment at: lib/profile/InstrProfiling.h:86
@@ +85,3 @@
+ */
+uint64_t __llvm_profile_gather_indirect_target_data(
+  __llvm_profile_indirect_target_data** IndirectTargetProfilingResults);
----------------
Similarly, make this interface more general.

http://reviews.llvm.org/D9009

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






More information about the llvm-commits mailing list