[HeapProf] Address post-review comments in instrumentation code

Addresses post-review comments from D85948, which can be found here:
https://reviews.llvm.org/rG7ed8124d46f9.

diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 8b89aac..ec77f68 100644 (file)
--- 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 (file)
--- 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<f_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 cce0eb5..0f51443 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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<ModuleHeapProfilerPass> {
 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 (file)
--- 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<InterestingMemoryAccess> isInterestingMemoryAccess(Instruction *I);
+  Optional<InterestingMemoryAccess>
+  isInterestingMemoryAccess(Instruction *I) const;
 
   void instrumentMop(Instruction *I, const DataLayout &DL,
                      InterestingMemoryAccess &Access);
@@ -321,7 +321,7 @@ void HeapProfiler::instrumentMemIntrinsic(MemIntrinsic *MI) {
 }
 
 Optional<InterestingMemoryAccess>
-HeapProfiler::isInterestingMemoryAccess(Instruction *I) {
+HeapProfiler::isInterestingMemoryAccess(Instruction *I) const {
   // Do not instrument the load fetching the dynamic shadow address.
   if (DynamicShadowOffset == I)
     return None;