tejohnson created this revision.
tejohnson added a reviewer: davidxl.
Herald added subscribers: wenlei, dang, hiraditya.
Herald added projects: clang, LLVM.
tejohnson requested review of this revision.

Similar to -fprofile-generate=, add -fmemory-profile= which takes a
directory path. This is passed down to LLVM via a new module flag
metadata. LLVM in turn provides this name to the runtime via the new
__memprof_profile_filename variable.

Additionally, always pass a default filename (in $cwd if a directory
name is not specified vi the = form of the option). This is also
consistent with the behavior of the PGO instrumentation. Since the
memory profiles will generally be fairly large, it doesn't make sense to
dump them to stderr. Also, importantly, the memory profiles will
eventually be dumped in a compact binary format, which is another reason
why it does not make sense to send these to stderr by default.

Depends on D89086 <https://reviews.llvm.org/D89086>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89087

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/memory-profile-filename.c
  clang/test/Driver/fmemprof.cpp
  llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
  llvm/test/Instrumentation/HeapProfiler/filename.ll

Index: llvm/test/Instrumentation/HeapProfiler/filename.ll
===================================================================
--- /dev/null
+++ llvm/test/Instrumentation/HeapProfiler/filename.ll
@@ -0,0 +1,15 @@
+; Test to ensure that the filename provided by clang in the module flags
+; metadata results in the expected __memprof_profile_filename insertion.
+
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -memprof -memprof-module -S | FileCheck --check-prefixes=CHECK %s
+
+define i32 @main() {
+entry:
+  ret i32 0
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MemProfProfileFilename", !"/tmp/memprof.profraw"}
+
+; CHECK: $__memprof_profile_filename = comdat any
+; CHECK: @__memprof_profile_filename = constant [21 x i8] c"/tmp/memprof.profraw\00", comdat
Index: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -60,6 +60,8 @@
 constexpr char MemProfShadowMemoryDynamicAddress[] =
     "__memprof_shadow_memory_dynamic_address";
 
+constexpr char MemProfFilenameVar[] = "__memprof_profile_filename";
+
 // Command-line flags.
 
 static cl::opt<bool> ClInsertVersionCheck(
@@ -486,6 +488,26 @@
   IRB.CreateStore(ShadowValue, ShadowAddr);
 }
 
+// Create the variable for the profile file name.
+void createProfileFileNameVar(Module &M) {
+  const MDString *MemProfFilename =
+      dyn_cast_or_null<MDString>(M.getModuleFlag("MemProfProfileFilename"));
+  if (!MemProfFilename)
+    return;
+  assert(!MemProfFilename->getString().empty() &&
+         "Unexpected MemProfProfileFilename metadata with empty string");
+  Constant *ProfileNameConst = ConstantDataArray::getString(
+      M.getContext(), MemProfFilename->getString(), true);
+  GlobalVariable *ProfileNameVar = new GlobalVariable(
+      M, ProfileNameConst->getType(), /*isConstant=*/true,
+      GlobalValue::WeakAnyLinkage, ProfileNameConst, MemProfFilenameVar);
+  Triple TT(M.getTargetTriple());
+  if (TT.supportsCOMDAT()) {
+    ProfileNameVar->setLinkage(GlobalValue::ExternalLinkage);
+    ProfileNameVar->setComdat(M.getOrInsertComdat(MemProfFilenameVar));
+  }
+}
+
 bool ModuleMemProfiler::instrumentModule(Module &M) {
   // Create a module constructor.
   std::string MemProfVersion = std::to_string(LLVM_MEM_PROFILER_VERSION);
@@ -500,6 +522,8 @@
   const uint64_t Priority = getCtorAndDtorPriority(TargetTriple);
   appendToGlobalCtors(M, MemProfCtorFunction, Priority);
 
+  createProfileFileNameVar(M);
+
   return true;
 }
 
Index: clang/test/Driver/fmemprof.cpp
===================================================================
--- clang/test/Driver/fmemprof.cpp
+++ clang/test/Driver/fmemprof.cpp
@@ -1,6 +1,10 @@
 // RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile %s -### 2>&1 | FileCheck %s
+// RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile=foo %s -### 2>&1 | FileCheck %s --check-prefix=DIR
 // RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile -fno-memory-profile %s -### 2>&1 | FileCheck %s --check-prefix=OFF
+// RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile=foo -fno-memory-profile %s -### 2>&1 | FileCheck %s --check-prefix=OFF
 // CHECK: "-cc1" {{.*}} "-fmemory-profile"
 // CHECK: ld{{.*}}libclang_rt.memprof{{.*}}libclang_rt.memprof_cxx
+// DIR: "-cc1" {{.*}} "-fmemory-profile=foo"
+// DIR: ld{{.*}}libclang_rt.memprof{{.*}}libclang_rt.memprof_cxx
 // OFF-NOT: "-fmemory-profile"
 // OFF-NOT: libclang_rt.memprof
Index: clang/test/CodeGen/memory-profile-filename.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memory-profile-filename.c
@@ -0,0 +1,12 @@
+// Test that we get the expected module flag metadata for the memory profile
+// filename.
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefix=NONE
+// RUN: %clang -target x86_64-linux-gnu -fmemory-profile -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DEFAULTNAME
+// RUN: %clang -target x86_64-linux-gnu -fmemory-profile=/tmp -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CUSTOMNAME
+int main (void) {
+  return 0;
+}
+
+// NONE-NOT: MemProfProfileFilename
+// DEFAULTNAME: !{i32 1, !"MemProfProfileFilename", !"memprof.profraw"}
+// CUSTOMNAME: !{i32 1, !"MemProfProfileFilename", !"/tmp/memprof.profraw"}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1035,7 +1035,15 @@
   Opts.ThinLinkBitcodeFile =
       std::string(Args.getLastArgValue(OPT_fthin_link_bitcode_EQ));
 
-  Opts.MemProf = Args.hasArg(OPT_fmemory_profile);
+  // The memory profile runtime appends the pid to make this name more unique.
+  std::string MemProfileBasename = "memprof.profraw";
+  if (Args.hasArg(OPT_fmemory_profile_EQ)) {
+    SmallString<128> Path(
+        std::string(Args.getLastArgValue(OPT_fmemory_profile_EQ)));
+    llvm::sys::path::append(Path, MemProfileBasename);
+    Opts.MemoryProfileOutput = std::string(Path);
+  } else if (Args.hasArg(OPT_fmemory_profile))
+    Opts.MemoryProfileOutput = MemProfileBasename;
 
   Opts.MSVolatile = Args.hasArg(OPT_fms_volatile);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4224,9 +4224,15 @@
   if (Args.getLastArg(options::OPT_save_temps_EQ))
     Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);
 
-  if (Args.hasFlag(options::OPT_fmemory_profile,
-                   options::OPT_fno_memory_profile, false))
-    Args.AddLastArg(CmdArgs, options::OPT_fmemory_profile);
+  auto *MemProfArg = Args.getLastArg(options::OPT_fmemory_profile,
+                                     options::OPT_fmemory_profile_EQ,
+                                     options::OPT_fno_memory_profile);
+  if (MemProfArg) {
+    if (MemProfArg->getOption().matches(options::OPT_fmemory_profile))
+      Args.AddLastArg(CmdArgs, options::OPT_fmemory_profile);
+    else if (MemProfArg->getOption().matches(options::OPT_fmemory_profile_EQ))
+      Args.AddLastArg(CmdArgs, options::OPT_fmemory_profile_EQ);
+  }
 
   // Embed-bitcode option.
   // Only white-listed flags below are allowed to be embedded.
Index: clang/lib/Driver/SanitizerArgs.cpp
===================================================================
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -869,6 +869,7 @@
                     D.CCCIsCXX();
 
   NeedsMemProfRt = Args.hasFlag(options::OPT_fmemory_profile,
+                                options::OPT_fmemory_profile_EQ,
                                 options::OPT_fno_memory_profile, false);
 
   // Finally, initialize the set of available and recoverable sanitizers.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -583,13 +583,20 @@
                               1);
   }
 
-  if (CodeGenOpts.CFProtectionBranch &&
-      Target.checkCFProtectionBranchSupported(getDiags())) {
-    // Indicate that we want to instrument branch control flow protection.
-    getModule().addModuleFlag(llvm::Module::Override, "cf-protection-branch",
+  if (CodeGenOpts.CFProtectionReturn &&
+      Target.checkCFProtectionReturnSupported(getDiags())) {
+    // Indicate that we want to instrument return control flow protection.
+    getModule().addModuleFlag(llvm::Module::Override, "cf-protection-return",
                               1);
   }
 
+  if (!CodeGenOpts.MemoryProfileOutput.empty()) {
+    llvm::LLVMContext &Ctx = TheModule.getContext();
+    getModule().addModuleFlag(
+        llvm::Module::Error, "MemProfProfileFilename",
+        llvm::MDString::get(Ctx, CodeGenOpts.MemoryProfileOutput));
+  }
+
   if (LangOpts.CUDAIsDevice && getTriple().isNVPTX()) {
     // Indicate whether __nvvm_reflect should be configured to flush denormal
     // floating point values to 0.  (This corresponds to its "__CUDA_FTZ"
Index: clang/lib/CodeGen/BackendUtil.cpp
===================================================================
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -673,7 +673,7 @@
   if (LangOpts.Coroutines)
     addCoroutinePassesToExtensionPoints(PMBuilder);
 
-  if (CodeGenOpts.MemProf) {
+  if (!CodeGenOpts.MemoryProfileOutput.empty()) {
     PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
                            addMemProfilerPasses);
     PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
@@ -1385,7 +1385,7 @@
       }
     }
 
-    if (CodeGenOpts.MemProf) {
+    if (!CodeGenOpts.MemoryProfileOutput.empty()) {
       MPM.addPass(createModuleToFunctionPassAdaptor(MemProfilerPass()));
       MPM.addPass(ModuleMemProfilerPass());
     }
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1003,6 +1003,9 @@
   Flags<[CC1Option]>;
 
 defm memory_profile : OptInFFlag<"memory-profile", "Enable", "Disable", " heap memory profiling">;
+def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
+    Group<f_Group>, Flags<[CC1Option, DriverOption]>, MetaVarName<"<directory>">,
+    HelpText<"Enable heap memory profiling and dump results into <directory>">;
 
 // Begin sanitizer flags. These should all be core options exposed in all driver
 // modes.
Index: clang/include/clang/Basic/CodeGenOptions.h
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -231,6 +231,9 @@
   /// Name of the profile file to use with -fprofile-sample-use.
   std::string SampleProfileFile;
 
+  /// Name of the profile file to use as output for with -fmemory-profile.
+  std::string MemoryProfileOutput;
+
   /// Name of the profile file to use as input for -fprofile-instr-use
   std::string ProfileInstrumentUsePath;
 
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -145,7 +145,6 @@
                                               ///< linker.
 CODEGENOPT(MergeAllConstants , 1, 1) ///< Merge identical constants.
 CODEGENOPT(MergeFunctions    , 1, 0) ///< Set when -fmerge-functions is enabled.
-CODEGENOPT(MemProf           , 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to