[PATCH] D124703: [memprof] Don't instrument PGO and other compiler inserted variables

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added a reviewer: snehasish.
Herald added subscribers: wenlei, hiraditya.
Herald added a project: All.
tejohnson requested review of this revision.
Herald added a project: LLVM.

Suppress instrumentation of PGO counter accesses, which is unnecessary
and costly. Also suppress accesses to other compiler inserted variables
starting with "__llvm". This is a slightly expanded variant of what is
done for tsan in shouldInstrumentReadWriteFromAddress.

While here, delay insertion of the dynamic shadow variable in the
function until we know whether there are any accesses to instrument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124703

Files:
  llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
  llvm/test/Instrumentation/HeapProfiler/skip-compiler-inserted.ll


Index: llvm/test/Instrumentation/HeapProfiler/skip-compiler-inserted.ll
===================================================================
--- /dev/null
+++ llvm/test/Instrumentation/HeapProfiler/skip-compiler-inserted.ll
@@ -0,0 +1,35 @@
+;; Test that we don't instrument loads to PGO counters or other
+;; compiler inserted variables.
+;
+; RUN: opt < %s -passes='function(memprof),module(memprof-module)' -S | FileCheck --check-prefixes=CHECK %s
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
+target triple = "x86_64-unknown-linux-gnu"
+
+$__profc__Z3foov = comdat nodeduplicate
+ at __profc__Z3foov = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+ at __llvm_gcov_ctr = internal global [1 x i64] zeroinitializer
+
+define void @_Z3foov() {
+entry:
+  ;; PGO counter update
+  %pgocount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3foov, i64 0, i64 0), align 8
+  %0 = add i64 %pgocount, 1
+  store i64 %0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3foov, i64 0, i64 0), align 8
+  ;; Gcov counter update
+  %gcovcount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__llvm_gcov_ctr, i64 0, i64 0), align 8
+  %1 = add i64 %gcovcount, 1
+  store i64 %1, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__llvm_gcov_ctr, i64 0, i64 0), align 8
+  ret void
+}
+
+;; We should not have any memory profile instrumentation
+; CHECK: define void @_Z3foov
+; CHECK-NEXT: entry:
+; CHECK-NEXT:  %pgocount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3foov, i64 0, i64 0)
+; CHECK-NEXT:  %0 = add i64 %pgocount, 1
+; CHECK-NEXT:  store i64 %0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3foov, i64 0, i64 0)
+; CHECK-NEXT:  %gcovcount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__llvm_gcov_ctr, i64 0, i64 0)
+; CHECK-NEXT:  %1 = add i64 %gcovcount, 1
+; CHECK-NEXT:  store i64 %1, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__llvm_gcov_ctr, i64 0, i64 0)
+; CHECK-NEXT:  ret void
Index: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -32,6 +32,7 @@
 #include "llvm/IR/Value.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
+#include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
@@ -408,6 +409,25 @@
   if (Access.Addr->isSwiftError())
     return None;
 
+  // Peel off GEPs and BitCasts.
+  auto *Addr = Access.Addr->stripInBoundsOffsets();
+
+  if (GlobalVariable *GV = dyn_cast<GlobalVariable>(Addr)) {
+    // Do not instrument PGO counter updates.
+    if (GV->hasSection()) {
+      StringRef SectionName = GV->getSection();
+      // Check if the global is in the PGO counters section.
+      auto OF = Triple(I->getModule()->getTargetTriple()).getObjectFormat();
+      if (SectionName.endswith(
+              getInstrProfSectionName(IPSK_cnts, OF, /*AddSegmentInfo=*/false)))
+        return None;
+    }
+
+    // Do not instrument accesses to LLVM internal variables.
+    if (GV->getName().startswith("__llvm"))
+      return None;
+  }
+
   const DataLayout &DL = I->getModule()->getDataLayout();
   Access.TypeSize = DL.getTypeStoreSizeInBits(Access.AccessTy);
   return Access;
@@ -613,8 +633,6 @@
 
   initializeCallbacks(*F.getParent());
 
-  FunctionModified |= insertDynamicShadowAtFunctionEntry(F);
-
   SmallVector<Instruction *, 16> ToInstrument;
 
   // Fill the set of memory operations to instrument.
@@ -625,6 +643,9 @@
     }
   }
 
+  if (!ToInstrument.empty())
+    FunctionModified |= insertDynamicShadowAtFunctionEntry(F);
+
   int NumInstrumented = 0;
   for (auto *Inst : ToInstrument) {
     if (ClDebugMin < 0 || ClDebugMax < 0 ||


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124703.426171.patch
Type: text/x-patch
Size: 4075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220429/5829e6ff/attachment.bin>