-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[MemProf] Split MemProfiler into Instrumentation and Use. #142811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MemProf] Split MemProfiler into Instrumentation and Use. #142811
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
3798a5b
to
0ba44d6
Compare
58e5e22
to
a8bea0a
Compare
- Add missing extern declarations for MemProf command line options - Remove duplicate function definition that conflicts with MemoryProfileInfo
a8bea0a
to
01c15f2
Compare
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Snehasish Kumar (snehasish) ChangesMost of the recent development on the MemProfiler has been on the Use part. The instrumentation has been quite stable for a while. As the complexity of the use grows (with undrifting, diagnostics etc) I figured it would be good to separate these two implementations. Patch is 60.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142811.diff 9 Files Affected:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index cd5fc48c4a22b..9cecf3c0f586e 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -69,7 +69,8 @@
#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
#include "llvm/Transforms/Instrumentation/KCFI.h"
#include "llvm/Transforms/Instrumentation/LowerAllowCheckPass.h"
-#include "llvm/Transforms/Instrumentation/MemProfiler.h"
+#include "llvm/Transforms/Instrumentation/MemProfInstrumentation.h"
+#include "llvm/Transforms/Instrumentation/MemProfUse.h"
#include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
#include "llvm/Transforms/Instrumentation/NumericalStabilitySanitizer.h"
#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
diff --git a/llvm/include/llvm/Transforms/Instrumentation/MemProfInstrumentation.h b/llvm/include/llvm/Transforms/Instrumentation/MemProfInstrumentation.h
new file mode 100644
index 0000000000000..ffc41993464fa
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Instrumentation/MemProfInstrumentation.h
@@ -0,0 +1,47 @@
+//===--- MemProfInstrumentation.h - Memory profiler instrumentation ----*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares the MemProf instrumentation pass classes.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROF_INSTRUMENTATION_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROF_INSTRUMENTATION_H
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+class Function;
+class Module;
+
+/// Public interface to the memory profiler pass for instrumenting code to
+/// profile memory accesses.
+///
+/// The profiler itself is a function pass that works by inserting various
+/// calls to the MemProfiler runtime library functions. The runtime library
+/// essentially replaces malloc() and free() with custom implementations that
+/// record data about the allocations.
+class MemProfilerPass : public PassInfoMixin<MemProfilerPass> {
+public:
+ explicit MemProfilerPass();
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+ static bool isRequired() { return true; }
+};
+
+/// Public interface to the memory profiler module pass for instrumenting code
+/// to profile memory allocations and accesses.
+class ModuleMemProfilerPass : public PassInfoMixin<ModuleMemProfilerPass> {
+public:
+ explicit ModuleMemProfilerPass();
+ PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
+ static bool isRequired() { return true; }
+};
+
+} // namespace llvm
+
+#endif
\ No newline at end of file
diff --git a/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h b/llvm/include/llvm/Transforms/Instrumentation/MemProfUse.h
similarity index 61%
rename from llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
rename to llvm/include/llvm/Transforms/Instrumentation/MemProfUse.h
index 169f757e580d2..91c6cdaecbce8 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/MemProfUse.h
@@ -1,4 +1,4 @@
-//===--------- Definition of the MemProfiler class --------------*- C++ -*-===//
+//===--------- MemProfUse.h - Memory profiler use pass ----*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,11 +6,11 @@
//
//===----------------------------------------------------------------------===//
//
-// This file declares the MemProfiler class.
+// This file declares the MemProfUsePass class and related utilities.
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFILER_H
-#define LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFILER_H
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFUSE_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFUSE_H
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/IR/PassManager.h"
@@ -19,7 +19,6 @@
#include <unordered_map>
namespace llvm {
-class Function;
class IndexedInstrProfReader;
class Module;
class TargetLibraryInfo;
@@ -28,29 +27,6 @@ namespace vfs {
class FileSystem;
} // namespace vfs
-/// Public interface to the memory profiler pass for instrumenting code to
-/// profile memory accesses.
-///
-/// The profiler itself is a function pass that works by inserting various
-/// calls to the MemProfiler runtime library functions. The runtime library
-/// essentially replaces malloc() and free() with custom implementations that
-/// record data about the allocations.
-class MemProfilerPass : public PassInfoMixin<MemProfilerPass> {
-public:
- explicit MemProfilerPass();
- PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
- static bool isRequired() { return true; }
-};
-
-/// Public interface to the memory profiler module pass for instrumenting code
-/// to profile memory allocations and accesses.
-class ModuleMemProfilerPass : public PassInfoMixin<ModuleMemProfilerPass> {
-public:
- explicit ModuleMemProfilerPass();
- PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
- static bool isRequired() { return true; }
-};
-
class MemProfUsePass : public PassInfoMixin<MemProfUsePass> {
public:
explicit MemProfUsePass(std::string MemoryProfileFile,
@@ -90,4 +66,4 @@ computeUndriftMap(Module &M, IndexedInstrProfReader *MemProfReader,
} // namespace memprof
} // namespace llvm
-#endif
+#endif
\ No newline at end of file
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index a6c59c1ca846e..4603eaff8ade9 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -240,7 +240,8 @@
#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
#include "llvm/Transforms/Instrumentation/KCFI.h"
#include "llvm/Transforms/Instrumentation/LowerAllowCheckPass.h"
-#include "llvm/Transforms/Instrumentation/MemProfiler.h"
+#include "llvm/Transforms/Instrumentation/MemProfInstrumentation.h"
+#include "llvm/Transforms/Instrumentation/MemProfUse.h"
#include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
#include "llvm/Transforms/Instrumentation/NumericalStabilitySanitizer.h"
#include "llvm/Transforms/Instrumentation/PGOCtxProfFlattening.h"
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 227390f557fda..452baf2da5125 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -76,7 +76,8 @@
#include "llvm/Transforms/Instrumentation/CGProfile.h"
#include "llvm/Transforms/Instrumentation/ControlHeightReduction.h"
#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
-#include "llvm/Transforms/Instrumentation/MemProfiler.h"
+#include "llvm/Transforms/Instrumentation/MemProfInstrumentation.h"
+#include "llvm/Transforms/Instrumentation/MemProfUse.h"
#include "llvm/Transforms/Instrumentation/PGOCtxProfFlattening.h"
#include "llvm/Transforms/Instrumentation/PGOCtxProfLowering.h"
#include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h"
diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt
index 0b863f6fef460..15fd421a41b0f 100644
--- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt
+++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt
@@ -6,7 +6,8 @@ add_llvm_component_library(LLVMInstrumentation
DataFlowSanitizer.cpp
GCOVProfiling.cpp
BlockCoverageInference.cpp
- MemProfiler.cpp
+ MemProfInstrumentation.cpp
+ MemProfUse.cpp
MemorySanitizer.cpp
NumericalStabilitySanitizer.cpp
IndirectCallPromotion.cpp
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/MemProfInstrumentation.cpp
new file mode 100644
index 0000000000000..ca6d91f7adb5d
--- /dev/null
+++ b/llvm/lib/Transforms/Instrumentation/MemProfInstrumentation.cpp
@@ -0,0 +1,658 @@
+//===- MemProfInstrumentation.cpp - memory allocation and access profiler
+// instrumentation ------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of MemProf. Memory accesses are instrumented
+// to increment the access count held in a shadow memory location, or
+// alternatively to call into the runtime. Memory intrinsic calls (memmove,
+// memcpy, memset) are changed to call the memory profiling runtime version
+// instead.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Instrumentation/MemProfInstrumentation.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/MemoryBuiltins.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Analysis/ValueTracking.h"
+#include "llvm/IR/Constant.h"
+#include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalValue.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Type.h"
+#include "llvm/IR/Value.h"
+#include "llvm/ProfileData/InstrProf.h"
+#include "llvm/ProfileData/MemProf.h"
+#include "llvm/ProfileData/MemProfCommon.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/TargetParser/Triple.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
+
+using namespace llvm;
+using namespace llvm::memprof;
+
+#define DEBUG_TYPE "memprof"
+
+constexpr int LLVM_MEM_PROFILER_VERSION = 1;
+
+// Size of memory mapped to a single shadow location.
+constexpr uint64_t DefaultMemGranularity = 64;
+
+// Size of memory mapped to a single histogram bucket.
+constexpr uint64_t HistogramGranularity = 8;
+
+// Scale from granularity down to shadow size.
+constexpr uint64_t DefaultShadowScale = 3;
+
+constexpr char MemProfModuleCtorName[] = "memprof.module_ctor";
+constexpr uint64_t MemProfCtorAndDtorPriority = 1;
+// On Emscripten, the system needs more than one priorities for constructors.
+constexpr uint64_t MemProfEmscriptenCtorAndDtorPriority = 50;
+constexpr char MemProfInitName[] = "__memprof_init";
+constexpr char MemProfVersionCheckNamePrefix[] =
+ "__memprof_version_mismatch_check_v";
+
+constexpr char MemProfShadowMemoryDynamicAddress[] =
+ "__memprof_shadow_memory_dynamic_address";
+
+constexpr char MemProfFilenameVar[] = "__memprof_profile_filename";
+
+constexpr char MemProfHistogramFlagVar[] = "__memprof_histogram";
+
+// Command-line flags.
+
+static cl::opt<bool> ClInsertVersionCheck(
+ "memprof-guard-against-version-mismatch",
+ cl::desc("Guard against compiler/runtime version mismatch."), cl::Hidden,
+ cl::init(true));
+
+// This flag may need to be replaced with -f[no-]memprof-reads.
+static cl::opt<bool> ClInstrumentReads("memprof-instrument-reads",
+ cl::desc("instrument read instructions"),
+ cl::Hidden, cl::init(true));
+
+static cl::opt<bool>
+ ClInstrumentWrites("memprof-instrument-writes",
+ cl::desc("instrument write instructions"), cl::Hidden,
+ cl::init(true));
+
+static cl::opt<bool> ClInstrumentAtomics(
+ "memprof-instrument-atomics",
+ cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
+ cl::init(true));
+
+static cl::opt<bool> ClUseCalls(
+ "memprof-use-callbacks",
+ cl::desc("Use callbacks instead of inline instrumentation sequences."),
+ cl::Hidden, cl::init(false));
+
+static cl::opt<std::string>
+ ClMemoryAccessCallbackPrefix("memprof-memory-access-callback-prefix",
+ cl::desc("Prefix for memory access callbacks"),
+ cl::Hidden, cl::init("__memprof_"));
+
+// These flags allow to change the shadow mapping.
+// The shadow mapping looks like
+// Shadow = ((Mem & mask) >> scale) + offset
+
+static cl::opt<int> ClMappingScale("memprof-mapping-scale",
+ cl::desc("scale of memprof shadow mapping"),
+ cl::Hidden, cl::init(DefaultShadowScale));
+
+static cl::opt<int>
+ ClMappingGranularity("memprof-mapping-granularity",
+ cl::desc("granularity of memprof shadow mapping"),
+ cl::Hidden, cl::init(DefaultMemGranularity));
+
+static cl::opt<bool> ClStack("memprof-instrument-stack",
+ cl::desc("Instrument scalar stack variables"),
+ cl::Hidden, cl::init(false));
+
+// Debug flags.
+
+static cl::opt<int> ClDebug("memprof-debug", cl::desc("debug"), cl::Hidden,
+ cl::init(0));
+
+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> ClHistogram("memprof-histogram",
+ cl::desc("Collect access count histograms"),
+ cl::Hidden, cl::init(false));
+
+static cl::opt<std::string>
+ MemprofRuntimeDefaultOptions("memprof-runtime-default-options",
+ cl::desc("The default memprof options"),
+ cl::Hidden, cl::init(""));
+
+// Instrumentation statistics
+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;
+ Granularity = ClHistogram ? HistogramGranularity : ClMappingGranularity;
+ Mask = ~(Granularity - 1);
+ }
+
+ int Scale;
+ int Granularity;
+ uint64_t Mask; // Computed as ~(Granularity-1)
+};
+
+static uint64_t getCtorAndDtorPriority(Triple &TargetTriple) {
+ return TargetTriple.isOSEmscripten() ? MemProfEmscriptenCtorAndDtorPriority
+ : MemProfCtorAndDtorPriority;
+}
+
+struct InterestingMemoryAccess {
+ Value *Addr = nullptr;
+ bool IsWrite;
+ Type *AccessTy;
+ Value *MaybeMask = nullptr;
+};
+
+/// Instrument the code in module to profile memory accesses.
+class MemProfiler {
+public:
+ MemProfiler(Module &M) {
+ C = &(M.getContext());
+ LongSize = M.getDataLayout().getPointerSizeInBits();
+ IntptrTy = Type::getIntNTy(*C, LongSize);
+ PtrTy = PointerType::getUnqual(*C);
+ }
+
+ /// If it is an interesting memory access, populate information
+ /// about the access and return a InterestingMemoryAccess struct.
+ /// Otherwise return std::nullopt.
+ std::optional<InterestingMemoryAccess>
+ isInterestingMemoryAccess(Instruction *I) const;
+
+ void instrumentMop(Instruction *I, const DataLayout &DL,
+ InterestingMemoryAccess &Access);
+ void instrumentAddress(Instruction *OrigIns, Instruction *InsertBefore,
+ Value *Addr, bool IsWrite);
+ void instrumentMaskedLoadOrStore(const DataLayout &DL, Value *Mask,
+ Instruction *I, Value *Addr, Type *AccessTy,
+ bool IsWrite);
+ void instrumentMemIntrinsic(MemIntrinsic *MI);
+ Value *memToShadow(Value *Shadow, IRBuilder<> &IRB);
+ bool instrumentFunction(Function &F);
+ bool maybeInsertMemProfInitAtFunctionEntry(Function &F);
+ bool insertDynamicShadowAtFunctionEntry(Function &F);
+
+private:
+ void initializeCallbacks(Module &M);
+
+ LLVMContext *C;
+ int LongSize;
+ Type *IntptrTy;
+ PointerType *PtrTy;
+ ShadowMapping Mapping;
+
+ // These arrays is indexed by AccessIsWrite
+ FunctionCallee MemProfMemoryAccessCallback[2];
+
+ FunctionCallee MemProfMemmove, MemProfMemcpy, MemProfMemset;
+ Value *DynamicShadowOffset = nullptr;
+};
+
+class ModuleMemProfiler {
+public:
+ ModuleMemProfiler(Module &M) { TargetTriple = M.getTargetTriple(); }
+
+ bool instrumentModule(Module &);
+
+private:
+ Triple TargetTriple;
+ ShadowMapping Mapping;
+ Function *MemProfCtorFunction = nullptr;
+};
+
+} // end anonymous namespace
+
+MemProfilerPass::MemProfilerPass() = default;
+
+PreservedAnalyses MemProfilerPass::run(Function &F,
+ AnalysisManager<Function> &AM) {
+ assert((!ClHistogram || ClMappingGranularity == DefaultMemGranularity) &&
+ "Memprof with histogram only supports default mapping granularity");
+ Module &M = *F.getParent();
+ MemProfiler Profiler(M);
+ if (Profiler.instrumentFunction(F))
+ return PreservedAnalyses::none();
+ return PreservedAnalyses::all();
+}
+
+ModuleMemProfilerPass::ModuleMemProfilerPass() = default;
+
+PreservedAnalyses ModuleMemProfilerPass::run(Module &M,
+ AnalysisManager<Module> &AM) {
+
+ ModuleMemProfiler Profiler(M);
+ if (Profiler.instrumentModule(M))
+ return PreservedAnalyses::none();
+ return PreservedAnalyses::all();
+}
+
+Value *MemProfiler::memToShadow(Value *Shadow, IRBuilder<> &IRB) {
+ // (Shadow & mask) >> scale
+ Shadow = IRB.CreateAnd(Shadow, Mapping.Mask);
+ Shadow = IRB.CreateLShr(Shadow, Mapping.Scale);
+ // (Shadow >> scale) | offset
+ assert(DynamicShadowOffset);
+ return IRB.CreateAdd(Shadow, DynamicShadowOffset);
+}
+
+// Instrument memset/memmove/memcpy
+void MemProfiler::instrumentMemIntrinsic(MemIntrinsic *MI) {
+ IRBuilder<> IRB(MI);
+ if (isa<MemTransferInst>(MI)) {
+ IRB.CreateCall(isa<MemMoveInst>(MI) ? MemProfMemmove : MemProfMemcpy,
+ {MI->getOperand(0), MI->getOperand(1),
+ IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
+ } else if (isa<MemSetInst>(MI)) {
+ IRB.CreateCall(
+ MemProfMemset,
+ {MI->getOperand(0),
+ IRB.CreateIntCast(MI->getOperand(1), IRB.getInt32Ty(), false),
+ IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
+ }
+ MI->eraseFromParent();
+}
+
+std::optional<InterestingMemoryAccess>
+MemProfiler::isInterestingMemoryAccess(Instruction *I) const {
+ // Do not instrument the load fetching the dynamic shadow address.
+ if (DynamicShadowOffset == I)
+ return std::nullopt;
+
+ InterestingMemoryAccess Access;
+
+ if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
+ if (!ClInstrumentReads)
+ return std::nullopt;
+ Access.IsWrite = false;
+ Access.AccessTy = LI->getType();
+ Access.Addr = LI->getPointerOperand();
+ } else if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
+ if (!ClInstrumentWrites)
+ return std::nullopt;
+ Access.IsWrite = true;
+ Access.AccessTy = SI->getValueOperand()->getType();
+ Access.Addr = SI->getPointerOperand();
+ } else if (AtomicRMWInst *RMW = dyn_cast<AtomicRMWInst>(I)) {
+ if (!ClInstrumentAtomics)
+ return std::nullopt;
+ Access.IsWrite = true;
+ Access.AccessTy = RMW->getValOperand()->getType();
+ Access.Addr = RMW->getPointerOperand();
+ } else if (AtomicCmpXchgInst *XCHG = dyn_cast<AtomicCmpXchgInst>(I)) {
+ if (!ClInstrumentAtomics)
+ return std::nullopt;
+ Access.IsWrite = true;
+ Access.AccessTy = XCHG->getCompareOperand()->getType();
+ Access.Addr = XCHG->getPointerOperand();
+ } else if (auto *CI = dyn_cast<CallInst>(I)) {
+ auto *F = CI->getCalledFunction(...
[truncated]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple comments below but otherwise lgtm
using namespace llvm; | ||
using namespace llvm::memprof; | ||
|
||
#define DEBUG_TYPE "memprof" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense for the 2 files to get different DEBUG_TYPE values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't thought of that but now that I do -- we are only going to run one of Instrumentation or Use at a time so it seems unnecessary to to have distinct debug types. Let me know if you feel strongly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for doing this!
@@ -90,4 +66,4 @@ computeUndriftMap(Module &M, IndexedInstrProfReader *MemProfReader, | |||
} // namespace memprof | |||
} // namespace llvm | |||
|
|||
#endif | |||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could we end with a newline here? A diff involving an unterminated line looks ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, fixed this and in other places in this PR.
21d8a4e
to
296c0c4
Compare
Merge activity
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/12020 Here is the relevant piece of the build log for the reference
|
Most of the recent development on the MemProfiler has been on the Use part. The instrumentation has been quite stable for a while. As the complexity of the use grows (with undrifting, diagnostics etc) I figured it would be good to separate these two implementations.