[PATCH] D27224: [Profile] Fix bad interaction of profile counter reset and value profile eviction

[David Li via Phabricator via llvm-commits]

davidxl created this revision.
davidxl added reviewers: vsk, xur.
davidxl added a subscriber: llvm-commits.

The problem is that value profile evict-or assumes value profile counts are non zero. The assumption is wrong once the counters are cleared (for value profile data, the counts are reset, but the target entries are still kept). When counter resetting and eviction is combined, the zero count entries will end up with large profile count due to wrapping.  When all entries are wrapped, adding new target will also lead to seg fault.


https://reviews.llvm.org/D27224

Files:
  lib/profile/InstrProfilingValue.c
  test/profile/Inputs/instrprof-value-prof-reset.c
  test/profile/instrprof-value-prof-reset.test


Index: test/profile/instrprof-value-prof-reset.test
===================================================================
--- test/profile/instrprof-value-prof-reset.test
+++ test/profile/instrprof-value-prof-reset.test
@@ -0,0 +1,16 @@
+// RUN: %clang_profgen -O2 -mllvm -enable-value-profiling=true -mllvm -vp-static-alloc=true -mllvm -vp-counters-per-site=3 -o %t %S/Inputs/instrprof-value-prof-reset.c
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-profdata show --all-functions -ic-targets  %t.profdata | FileCheck  %S/Inputs/instrprof-value-prof-reset.c
+
+// IR level instrumentation
+// RUN: %clang_pgogen -O2 -mllvm -disable-vp=false -mllvm -vp-static-alloc=true  -mllvm -vp-counters-per-site=3 -o %t.ir  %S/Inputs/instrprof-value-prof-reset.c
+// RUN: env LLVM_PROFILE_FILE=%t.ir.profraw %run %t.ir
+// RUN: llvm-profdata merge -o %t.ir.profdata %t.ir.profraw
+// RUN: llvm-profdata show --all-functions -ic-targets  %t.ir.profdata | FileCheck  %S/Inputs/instrprof-value-prof-reset.c
+
+// IR level instrumentation, dynamic allocation
+// RUN: %clang_pgogen -O2 -mllvm -disable-vp=false -mllvm -vp-static-alloc=false -o %t.ir.dyn  %S/Inputs/instrprof-value-prof-reset.c
+// RUN: env LLVM_PROFILE_FILE=%t.ir.dyn.profraw %run %t.ir.dyn
+// RUN: llvm-profdata merge -o %t.ir.dyn.profdata %t.ir.dyn.profraw
+// RUN: llvm-profdata show --all-functions -ic-targets  %t.ir.dyn.profdata | FileCheck  %S/Inputs/instrprof-value-prof-reset.c
Index: test/profile/Inputs/instrprof-value-prof-reset.c
===================================================================
--- test/profile/Inputs/instrprof-value-prof-reset.c
+++ test/profile/Inputs/instrprof-value-prof-reset.c
@@ -0,0 +1,30 @@
+void callee_0() {}
+void callee_1() {}
+void callee_2() {}
+
+void *CalleeAddrs[] = {callee_0, callee_1, callee_2, callee_2, callee_2};
+extern void lprofSetMaxValsPerSite(unsigned);
+extern void __llvm_profile_reset_counters();
+
+typedef void (*FPT)(void);
+
+
+// Testing value profiling eviction algorithm.
+FPT getCalleeFunc(int I) { return CalleeAddrs[I]; }
+
+int main() {
+  int I;
+
+  // First fill up two value profile entries with two targets
+  lprofSetMaxValsPerSite(2);
+
+  for (I = 0; I < 5; I++) {
+    if (I == 2) {
+      __llvm_profile_reset_counters();
+    }
+    // CHECK:  callee_2, 3
+    // ChECK-NEXT: callee_1, 0
+    FPT FP = getCalleeFunc(I);
+    FP();
+  }
+}
Index: lib/profile/InstrProfilingValue.c
===================================================================
--- lib/profile/InstrProfilingValue.c
+++ lib/profile/InstrProfilingValue.c
@@ -192,7 +192,7 @@
      * the runtime can wipe out more than one lowest count entries
      * to give space for hot targets.
      */
-    if (!(--MinCountVNode->Count)) {
+    if (!MinCountVNode->Count || !(--MinCountVNode->Count)) {
       CurVNode = MinCountVNode;
       CurVNode->Value = TargetValue;
       CurVNode->Count++;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27224.79628.patch
Type: text/x-patch
Size: 2971 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161129/20b2b29c/attachment.bin>