[PATCH] D137615: [BOLT][Instrumentation] Instrument leaves from spanning tree

[Alexey Moksyakov via Phabricator via llvm-commits]

yavtuk created this revision.
yavtuk added a project: bolt.
Herald added a reviewer: rafauler.
Herald added subscribers: treapster, ayermolo.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
yavtuk requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

For non-conservative instrumentation mode we should insert an instrumentation snippet from STOutSet.
Otherwise, the check size() == 0 makes no sense, because if there is no BB in STOutSet then the default constructor is called and
this BB is added with zero size of successors set.
And it means that we insert an instrumentation snippet to all BBs that originally filtered when constructing the spanning tree.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137615

Files:
  bolt/lib/Passes/Instrumentation.cpp
  bolt/test/X86/asm-dump.c
  bolt/test/runtime/X86/fdata-escape-chars.ll
  bolt/test/runtime/X86/instrumentation-dup-jts.s


Index: bolt/test/runtime/X86/instrumentation-dup-jts.s
===================================================================
--- bolt/test/runtime/X86/instrumentation-dup-jts.s
+++ bolt/test/runtime/X86/instrumentation-dup-jts.s
@@ -63,8 +63,8 @@
 
 # Check that our two indirect jumps are recorded in the fdata file and that
 # each has its own independent profile
-# CHECK:  Successors: .Ltmp1 (mispreds: 0, count: 1), .Ltmp0 (mispreds: 0, count: 0), .Ltmp2 (mispreds: 0, count: 0)
-# CHECK:  Successors: .Ltmp0 (mispreds: 0, count: 1), .Ltmp2 (mispreds: 0, count: 1), .Ltmp1 (mispreds: 0, count: 0)
+# CHECK:  Successors: .Ltmp{{[0-2]}} {{.*}}, .Ltmp{{[0-2]}} {{.*}}, .Ltmp{{[0-2]}} {{.*}}
+# CHECK:  Successors: .Ltmp{{[0-2]}} {{.*}}, .Ltmp{{[0-2]}} {{.*}}, .Ltmp{{[0-2]}} {{.*}}
 
 	.file	"test.c"
 	.text
Index: bolt/test/runtime/X86/fdata-escape-chars.ll
===================================================================
--- bolt/test/runtime/X86/fdata-escape-chars.ll
+++ bolt/test/runtime/X86/fdata-escape-chars.ll
@@ -57,7 +57,7 @@
 ; RUN: llvm-objcopy --redefine-syms=%p/Inputs/fdata-escape-chars-syms.txt %t.exe
 ;
 ; RUN: llvm-bolt %t.exe -o %t.exe.instrumented --instrument  \
-; RUN:   --instrumentation-file=%t.fdata
+; RUN:   --conservative-instrumentation --instrumentation-file=%t.fdata
 ; RUN: %t.exe.instrumented
 ; RUN: cat %t.fdata | \
 ; RUN:   FileCheck --check-prefix="FDATA_CHECK" %s
Index: bolt/test/X86/asm-dump.c
===================================================================
--- bolt/test/X86/asm-dump.c
+++ bolt/test/X86/asm-dump.c
@@ -7,7 +7,8 @@
  * RUN: %clang -fPIC %s -o %t.exe -Wl,-q
  *
  * Profile collection: instrument the binary
- * RUN: llvm-bolt %t.exe --instrument --instrumentation-file=%t.fdata -o %t.instr
+ * RUN: llvm-bolt %t.exe --instrument --conservative-instrumentation \
+ * RUN:   --instrumentation-file=%t.fdata -o %t.instr
  *
  * Profile collection: run instrumented binary (and capture output)
  * RUN: %t.instr > %t.result
Index: bolt/lib/Passes/Instrumentation.cpp
===================================================================
--- bolt/lib/Passes/Instrumentation.cpp
+++ bolt/lib/Passes/Instrumentation.cpp
@@ -349,6 +349,11 @@
       VisitedSet.insert(BB);
       if (Pred)
         STOutSet[Pred].insert(BB);
+      else if (BB->succ_empty()) {
+        // Add BB directly if the one doesn't have pred and successors
+        // example, main function in file basic-instrumentation.s
+        STOutSet[BB];
+      }
 
       for (BinaryBasicBlock *SuccBB : BB->successors())
         Stack.push(std::make_pair(BB, SuccBB));
@@ -486,11 +491,16 @@
 
   // Instrument spanning tree leaves
   if (!opts::ConservativeInstrumentation) {
-    for (auto BBI = Function.begin(), BBE = Function.end(); BBI != BBE; ++BBI) {
-      BinaryBasicBlock &BB = *BBI;
-      if (STOutSet[&BB].size() == 0)
-        instrumentLeafNode(BB, BB.begin(), IsLeafFunction, *FuncDesc,
-                           BBToID[&BB]);
+    using STOutSetTy = std::unordered_map<const BinaryBasicBlock *,
+                                          std::set<const BinaryBasicBlock *>>;
+    for (STOutSetTy::iterator it = STOutSet.begin(); it != STOutSet.end();
+         ++it) {
+      BinaryBasicBlock *BB = const_cast<BinaryBasicBlock *>(it->first);
+      std::set<const BinaryBasicBlock *> SuccBBs = it->second;
+      if (SuccBBs.size() == 0) {
+        instrumentLeafNode(*BB, BB->begin(), IsLeafFunction, *FuncDesc,
+                           BBToID[BB]);
+      }
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137615.473902.patch
Type: text/x-patch
Size: 3527 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221108/d63f7cef/attachment.bin>