[MemProf] Don't instrument stack accesses unless requested

[tejohnson]

Summary

Skip stack accesses unless requested, as the memory profiler runtime
does not currently look at or report accesses for these addresses.

[MaskRay]

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

459

the coding standard prefer pre-increment.

llvm/test/Instrumentation/HeapProfiler/stack.ll

5

Legacy PM tests are being deleted. For new tests, just avoid them.

[tejohnson]

Address comments

[tejohnson]

llvm/test/Instrumentation/HeapProfiler/stack.ll

5

I see. Just skimmed the legacy PM deprecation email and saw note about this. Might be worth a wider announcement?

[snehasish]

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

125

Perhaps call this memprof-instrument-stack, the usage will be clear from the name itself?

Also I think moving this to L110 is better since the comment at L111 seems to indicate all flags following it are debug flags.

[tejohnson]

Address comments

[snehasish]

LGTM

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Instrumentation/MemProfiler.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
static cl::opt<std::string> ClDebugFunc("memprof-debug-func", cl::Hidden,
                                         cl::desc("Debug func"));
 
 static cl::opt<int> ClDebugMin("memprof-debug-min", cl::desc("Debug min inst"),
                                cl::Hidden, cl::init(-1));
 
 static cl::opt<int> ClDebugMax("memprof-debug-max", cl::desc("Debug max inst"),
                                cl::Hidden, cl::init(-1));
 
+static cl::opt<bool> ClStack("memprof-stack",
+                             cl::desc("Instrument scalar stack variables"),
+                             cl::Hidden, cl::init(false));
+
 STATISTIC(NumInstrumentedReads, "Number of instrumented reads");
 STATISTIC(NumInstrumentedWrites, "Number of instrumented writes");
+STATISTIC(NumSkippedStackReads, "Number of non-instrumented stack reads");
+STATISTIC(NumSkippedStackWrites, "Number of non-instrumented stack writes");
 
 namespace {
 
 /// This struct defines the shadow mapping using the rule:
 ///   shadow = ((mem & mask) >> Scale) ADD DynamicShadowOffset.
 struct ShadowMapping {
   ShadowMapping() {
     Scale = ClMappingScale;
InstrumentedAddress =
         IRB.CreateGEP(VTy, Addr, {Zero, ConstantInt::get(IntptrTy, Idx)});
     instrumentAddress(I, InsertBefore, InstrumentedAddress, ElemTypeSize,
                       IsWrite);
   }
 }
 
 void MemProfiler::instrumentMop(Instruction *I, const DataLayout &DL,
                                 InterestingMemoryAccess &Access) {
+  // Skip instrumentation of stack accesses unless requested.
+  if (!ClStack && isa<AllocaInst>(getUnderlyingObject(Access.Addr))) {
+    if (Access.IsWrite)
+      ++NumSkippedStackWrites;
+    else
+      ++NumSkippedStackReads;
+    return;
+  }
+
   if (Access.IsWrite)
     NumInstrumentedWrites++;
   else
     NumInstrumentedReads++;
 
   if (Access.MaybeMask) {
     instrumentMaskedLoadOrStore(DL, Access.MaybeMask, I, Access.Addr,
                                 Access.Alignment, Access.TypeSize,

llvm/test/Instrumentation/HeapProfiler/stack.ll

+; Test optional annotation of stack addresses.
+;
+; RUN: opt < %s -passes='function(memprof),module(memprof-module)' -S | FileCheck --check-prefixes=CHECK %s
+; RUN: opt < %s -passes='function(memprof),module(memprof-module)' -memprof-stack -S | FileCheck --check-prefixes=CHECK,STACK %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"
+
+define i32 @test_stack_load() {
+entry:
+  %x = alloca i32, align 4
+  %tmp1 = load i32, i32* %x, align 4
+  ret i32 %tmp1
+}
+; CHECK-LABEL: @test_stack_load
+; CHECK:         %[[SHADOW_OFFSET:[^ ]*]] = load i64, i64* @__memprof_shadow_memory_dynamic_address
+; CHECK-NEXT:  %x = alloca i32
+; STACK-NEXT:    %[[LOAD_ADDR:[^ ]*]] = ptrtoint i32* %x to i64
+; STACK-NEXT:    %[[MASKED_ADDR:[^ ]*]] = and i64 %[[LOAD_ADDR]], -64
+; STACK-NEXT:    %[[SHIFTED_ADDR:[^ ]*]] = lshr i64 %[[MASKED_ADDR]], 3
+; STACK-NEXT:    add i64 %[[SHIFTED_ADDR]], %[[SHADOW_OFFSET]]
+; STACK-NEXT:    %[[LOAD_SHADOW_PTR:[^ ]*]] = inttoptr
+; STACK-NEXT:    %[[LOAD_SHADOW:[^ ]*]] = load i64, i64* %[[LOAD_SHADOW_PTR]]
+; STACK-NEXT:    %[[NEW_SHADOW:[^ ]*]] = add i64 %[[LOAD_SHADOW]], 1
+; STACK-NEXT:    store i64 %[[NEW_SHADOW]], i64* %[[LOAD_SHADOW_PTR]]
+; The actual load.
+; CHECK-NEXT:    %tmp1 = load i32, i32* %x
+; CHECK-NEXT:    ret i32 %tmp1
+
+define void @test_stack_store() {
+entry:
+  %x = alloca i32, align 4
+  store i32 1, i32* %x, align 4
+  ret void
+}
+; CHECK-LABEL: @test_stack_store
+; CHECK:         %[[SHADOW_OFFSET:[^ ]*]] = load i64, i64* @__memprof_shadow_memory_dynamic_address
+; CHECK-NEXT:  %x = alloca i32
+; STACK-NEXT:    %[[STORE_ADDR:[^ ]*]] = ptrtoint i32* %x to i64
+; STACK-NEXT:    %[[MASKED_ADDR:[^ ]*]] = and i64 %[[STORE_ADDR]], -64
+; STACK-NEXT:    %[[SHIFTED_ADDR:[^ ]*]] = lshr i64 %[[MASKED_ADDR]], 3
+; STACK-NEXT:    add i64 %[[SHIFTED_ADDR]], %[[SHADOW_OFFSET]]
+; STACK-NEXT:    %[[STORE_SHADOW_PTR:[^ ]*]] = inttoptr
+; STACK-NEXT:    %[[STORE_SHADOW:[^ ]*]] = load i64, i64* %[[STORE_SHADOW_PTR]]
+; STACK-NEXT:    %[[NEW_SHADOW:[^ ]*]] = add i64 %[[STORE_SHADOW]], 1
+; STACK-NEXT:    store i64 %[[NEW_SHADOW]], i64* %[[STORE_SHADOW_PTR]]
+; The actual store.
+; CHECK-NEXT:    store i32 1, i32* %x
+; CHECK-NEXT:    ret void