From 45c3560384814d04c9813e644efa8e2155ecae52 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 3 Sep 2020 23:29:21 -0700 Subject: [PATCH] [HeapProf] Address post-review comments in instrumentation code Addresses post-review comments from D85948, which can be found here: https://reviews.llvm.org/rG7ed8124d46f9. --- clang/include/clang/Basic/CodeGenOptions.def | 2 +- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/SanitizerArgs.cpp | 4 ++-- clang/lib/Driver/ToolChains/Clang.cpp | 5 +++-- clang/lib/Frontend/CompilerInvocation.cpp | 2 +- clang/test/Driver/fmemprof.cpp | 8 ++++---- llvm/include/llvm/Transforms/Instrumentation/HeapProfiler.h | 4 +--- llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp | 8 ++++---- 8 files changed, 17 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index 8b89aac..ec77f68 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -145,7 +145,7 @@ CODEGENOPT(IncrementalLinkerCompatible, 1, 0) ///< Emit an object file which can ///< linker. CODEGENOPT(MergeAllConstants , 1, 1) ///< Merge identical constants. CODEGENOPT(MergeFunctions , 1, 0) ///< Set when -fmerge-functions is enabled. -CODEGENOPT(HeapProf , 1, 0) ///< Set when -fmemprof is enabled. +CODEGENOPT(HeapProf , 1, 0) ///< Set when -fmemory-profile is enabled. CODEGENOPT(MSVolatile , 1, 0) ///< Set when /volatile:ms is enabled. CODEGENOPT(NoCommon , 1, 0) ///< Set when -fno-common or C++ is enabled. CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 9121926..5f1668e 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -995,7 +995,7 @@ defm cxx_static_destructors : OptOutFFlag<"c++-static-destructors", "", def fsymbol_partition_EQ : Joined<["-"], "fsymbol-partition=">, Group, Flags<[CC1Option]>; -defm memprof : OptInFFlag<"memprof", "Enable", "Disable", " heap memory profiling">; +defm memory_profile : OptInFFlag<"memory-profile", "Enable", "Disable", " heap memory profiling">; // Begin sanitizer flags. These should all be core options exposed in all driver // modes. diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index cce0eb55..0f51443 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -866,8 +866,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, LinkCXXRuntimes) || D.CCCIsCXX(); - NeedsHeapProfRt = - Args.hasFlag(options::OPT_fmemprof, options::OPT_fno_memprof, false); + NeedsHeapProfRt = Args.hasFlag(options::OPT_fmemory_profile, + options::OPT_fno_memory_profile, false); // Finally, initialize the set of available and recoverable sanitizers. Sanitizers.Mask |= Kinds; diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index bd5a89c..1680f2a 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4224,8 +4224,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (Args.getLastArg(options::OPT_save_temps_EQ)) Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ); - if (Args.hasFlag(options::OPT_fmemprof, options::OPT_fno_memprof, false)) - Args.AddLastArg(CmdArgs, options::OPT_fmemprof); + if (Args.hasFlag(options::OPT_fmemory_profile, + options::OPT_fno_memory_profile, false)) + Args.AddLastArg(CmdArgs, options::OPT_fmemory_profile); // Embed-bitcode option. // Only white-listed flags below are allowed to be embedded. diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 9143dd6..fbccff1 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -1033,7 +1033,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, Opts.ThinLinkBitcodeFile = std::string(Args.getLastArgValue(OPT_fthin_link_bitcode_EQ)); - Opts.HeapProf = Args.hasArg(OPT_fmemprof); + Opts.HeapProf = Args.hasArg(OPT_fmemory_profile); Opts.MSVolatile = Args.hasArg(OPT_fms_volatile); diff --git a/clang/test/Driver/fmemprof.cpp b/clang/test/Driver/fmemprof.cpp index 0490678..a2b740e 100644 --- a/clang/test/Driver/fmemprof.cpp +++ b/clang/test/Driver/fmemprof.cpp @@ -1,6 +1,6 @@ -// RUN: %clangxx -target x86_64-linux-gnu -fmemprof %s -### 2>&1 | FileCheck %s -// RUN: %clangxx -target x86_64-linux-gnu -fmemprof -fno-memprof %s -### 2>&1 | FileCheck %s --check-prefix=OFF -// CHECK: "-cc1" {{.*}} "-fmemprof" +// RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile %s -### 2>&1 | FileCheck %s +// RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile -fno-memory-profile %s -### 2>&1 | FileCheck %s --check-prefix=OFF +// CHECK: "-cc1" {{.*}} "-fmemory-profile" // CHECK: ld{{.*}}libclang_rt.heapprof{{.*}}libclang_rt.heapprof_cxx -// OFF-NOT: "-fmemprof" +// OFF-NOT: "-fmemory-profile" // OFF-NOT: libclang_rt.heapprof diff --git a/llvm/include/llvm/Transforms/Instrumentation/HeapProfiler.h b/llvm/include/llvm/Transforms/Instrumentation/HeapProfiler.h index af905bb..2194361 100644 --- a/llvm/include/llvm/Transforms/Instrumentation/HeapProfiler.h +++ b/llvm/include/llvm/Transforms/Instrumentation/HeapProfiler.h @@ -1,4 +1,4 @@ -//===--------- Definition of the HeapProfiler class ---------*- C++ -*-===// +//===--------- Definition of the HeapProfiler class -------------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -38,8 +38,6 @@ class ModuleHeapProfilerPass : public PassInfoMixin { public: explicit ModuleHeapProfilerPass(); PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); - -private: }; // Insert HeapProfiler instrumentation diff --git a/llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp b/llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp index 6372dfd..5f8671d 100644 --- a/llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp +++ b/llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp @@ -1,5 +1,4 @@ -//===- HeapProfiler.cpp - heap allocation and access profiler -//--------------===// +//===- HeapProfiler.cpp - heap allocation and access profiler -------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -164,7 +163,8 @@ public: /// If it is an interesting memory access, populate information /// about the access and return a InterestingMemoryAccess struct. /// Otherwise return None. - Optional isInterestingMemoryAccess(Instruction *I); + Optional + isInterestingMemoryAccess(Instruction *I) const; void instrumentMop(Instruction *I, const DataLayout &DL, InterestingMemoryAccess &Access); @@ -321,7 +321,7 @@ void HeapProfiler::instrumentMemIntrinsic(MemIntrinsic *MI) { } Optional -HeapProfiler::isInterestingMemoryAccess(Instruction *I) { +HeapProfiler::isInterestingMemoryAccess(Instruction *I) const { // Do not instrument the load fetching the dynamic shadow address. if (DynamicShadowOffset == I) return None; -- 2.7.4