[MemProf] Pass down memory profile name with optional path from clang

[tejohnson]

Summary

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.

[tejohnson]

As discussed in review for D89086, move test changes to this patch since they are only required here.

[davidxl]

There should be a related LLVM side of changes. Is it in a different patch?

clang/lib/CodeGen/CodeGenModule.cpp

586–589

Is this change relevant?

595

Is there a need for a module flag? PGO passes an InstrProfOptions typed object to the instrumentation pass. The option object has the directory info.

[davidxl]

longer term, the profile will be dumped into PGO's raw file, so for now is there a need for a user level option? should an internal option good enough?

[tejohnson]

In D89087#2359962, @davidxl wrote:

There should be a related LLVM side of changes. Is it in a different patch?

That's here too. See the change in llvm/lib/Transforms/Instrumentation/MemProfiler.cpp and a related test (they are near the bottom of the patch below all the clang tests).

In D89087#2359977, @davidxl wrote:

longer term, the profile will be dumped into PGO's raw file, so for now is there a need for a user level option? should an internal option good enough?

I think it would be good to keep an option to specify output for this profile independently. Both for users that want to only collect a memory profile in a single build/run, and for users that may want to keep it separate for some reason (and us for now).

clang/lib/CodeGen/CodeGenModule.cpp

586–589

Woops, looks like a bad merge! Will fix.

595

I was trying to avoid going the route PGO uses to pass that down, as it is different between the old and new PM and I think more complex than necessary as a result.

[tejohnson]

Rebase and fix bad merge

[davidxl]

lgtm

[MaskRay]

clang/include/clang/Driver/Options.td

1007

DriverOption has been renamed to NoXarchOption (which is just used to provide a diagnostic for -Xarch, which may be removed in the future). Please remove the flag

clang/lib/Driver/ToolChains/Clang.cpp

4231

if not match options::OPT_fno_memory_profile, render(Args, CmdArgs)

clang/lib/Frontend/CompilerInvocation.cpp

1039

const StringLiteral

1046

std::string(MemProfileBasename)

[tejohnson]

clang/lib/Frontend/CompilerInvocation.cpp

1046

This change doesn't seem to be needed?

[tejohnson]

Address comments

clang/include/clang/Basic/CodeGenOptions.h

public:
 
   /// Name of the profile file to use as output for -fprofile-instr-generate,
   /// -fprofile-generate, and -fcs-profile-generate.
   std::string InstrProfileOutput;
 
   /// 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;
 
   /// Name of the profile remapping file to apply to the profile data supplied
   /// by -fprofile-sample-use or -fprofile-instr-use.
   std::string ProfileRemappingFile;
 
   /// Name of the function summary index file to use for ThinLTO function

clang/include/clang/Basic/CodeGenOptions.def

CODEGENOPT(EnableSplitLTOUnit, 1, 0) ///< Enable LTO unit splitting to support
              /// CFI and traditional whole program
              /// devirtualization that require whole
              /// program IR support.
 CODEGENOPT(IncrementalLinkerCompatible, 1, 0) ///< Emit an object file which can
                                               ///< be used with an incremental
                                               ///< 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
                                        ///< enabled.
 CODEGENOPT(NoExecStack       , 1, 0) ///< Set when -Wa,--noexecstack is enabled.
 CODEGENOPT(FatalWarnings     , 1, 0) ///< Set when -Wa,--fatal-warnings is
                                      ///< enabled.
 CODEGENOPT(NoWarn            , 1, 0) ///< Set when -Wa,--no-warn is enabled.

clang/include/clang/Driver/Options.td

 defm keep_static_consts : OptInFFlag<"keep-static-consts", "Keep", "Don't keep", " static const variables if unused", [DriverOption]>;
 defm fixed_point : OptInFFlag<"fixed-point", "Enable", "Disable", " fixed point types">;
 defm cxx_static_destructors : OptOutFFlag<"c++-static-destructors", "",
   "Disable C++ static destructor registration">;
 def fsymbol_partition_EQ : Joined<["-"], "fsymbol-partition=">, Group<f_Group>,
   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.
 let Flags = [CC1Option, CoreOption] in {
 
 def fsanitize_EQ : CommaJoined<["-"], "fsanitize=">, Group<f_clang_Group>,
                    MetaVarName<"<check>">,
                    HelpText<"Turn on runtime checks for various forms of undefined "

clang/lib/CodeGen/BackendUtil.cpp

PMBuilder.addExtension(PassManagerBuilder::EP_ModuleOptimizerEarly,
                            addObjCARCAPElimPass);
     PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
                            addObjCARCOptPass);
   }
 
   if (LangOpts.Coroutines)
     addCoroutinePassesToExtensionPoints(PMBuilder);
 
-  if (CodeGenOpts.MemProf) {
+  if (!CodeGenOpts.MemoryProfileOutput.empty()) {
     PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
                            addMemProfilerPasses);
     PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
                            addMemProfilerPasses);
   }
 
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
     PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
if (CodeGenOpts.OptimizationLevel == 0) {
         MPM.addPass(CanonicalizeAliasesPass());
         MPM.addPass(NameAnonGlobalPass());
       } else {
         MPM = PB.buildPerModuleDefaultPipeline(Level,
                                                CodeGenOpts.DebugPassManager);
       }
     }
 
-    if (CodeGenOpts.MemProf) {
+    if (!CodeGenOpts.MemoryProfileOutput.empty()) {
       MPM.addPass(createModuleToFunctionPassAdaptor(MemProfilerPass()));
       MPM.addPass(ModuleMemProfilerPass());
     }
 
     if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
       bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
       MPM.addPass(HWAddressSanitizerPass(
           /*CompileKernel=*/false, Recover));

clang/lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::Release() {
 
   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.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"
     // property.)
     getModule().addModuleFlag(llvm::Module::Override, "nvvm-reflect-ftz",
                               CodeGenOpts.FP32DenormalMode.Output !=
                                   llvm::DenormalMode::IEEE);
   }

clang/lib/Driver/SanitizerArgs.cpp

SanitizerArgs::SanitizerArgs(const ToolChain &TC,
 
   // Parse -link-cxx-sanitizer flag.
   LinkCXXRuntimes = Args.hasArg(options::OPT_fsanitize_link_cxx_runtime,
                                 options::OPT_fno_sanitize_link_cxx_runtime,
                                 LinkCXXRuntimes) ||
                     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.
   Sanitizers.Mask |= Kinds;
   RecoverableSanitizers.Mask |= RecoverableKinds;
   TrapSanitizers.Mask |= TrappingKinds;
   assert(!(RecoverableKinds & TrappingKinds) &&
          "Overlap between recoverable and trapping sanitizers");

clang/lib/Driver/ToolChains/Clang.cpp

void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   if (Args.getLastArg(options::OPT_fthin_link_bitcode_EQ))
     Args.AddLastArg(CmdArgs, options::OPT_fthin_link_bitcode_EQ);
 
   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))
+  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.
   if (C.getDriver().embedBitcodeInObject() && !C.getDriver().isUsingLTO() &&
       (isa<BackendJobAction>(JA) || isa<AssembleJobAction>(JA))) {
     // Add flags implied by -fembed-bitcode.
     Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
     // Disable all llvm IR level optimizations.

clang/lib/Frontend/CompilerInvocation.cpp

if (Arg *A = Args.getLastArg(OPT_save_temps_EQ))
     Opts.SaveTempsFilePrefix =
         llvm::StringSwitch<std::string>(A->getValue())
             .Case("obj", FrontendOpts.OutputFile)
             .Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str());
 
   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);
 
   Opts.VectorizeLoop = Args.hasArg(OPT_vectorize_loops);
   Opts.VectorizeSLP = Args.hasArg(OPT_vectorize_slp);
 
   Opts.PreferVectorWidth =
       std::string(Args.getLastArgValue(OPT_mprefer_vector_width_EQ));

clang/test/CodeGen/memory-profile-filename.c

+// 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"}

clang/test/Driver/fmemprof.cpp

 // 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

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

 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";
+
 // 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.
void MemProfiler::instrumentAddress(Instruction *OrigIns,
   Value *ShadowPtr = memToShadow(AddrLong, IRB);
   Value *ShadowAddr = IRB.CreateIntToPtr(ShadowPtr, ShadowPtrTy);
   Value *ShadowValue = IRB.CreateLoad(ShadowTy, ShadowAddr);
   Value *Inc = ConstantInt::get(Type::getInt64Ty(*C), 1);
   ShadowValue = IRB.CreateAdd(ShadowValue, Inc);
   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);
   std::string VersionCheckName =
       ClInsertVersionCheck ? (MemProfVersionCheckNamePrefix + MemProfVersion)
                            : "";
   std::tie(MemProfCtorFunction, std::ignore) =
       createSanitizerCtorAndInitFunctions(M, MemProfModuleCtorName,
                                           MemProfInitName, /*InitArgTypes=*/{},
                                           /*InitArgs=*/{}, VersionCheckName);
 
   const uint64_t Priority = getCtorAndDtorPriority(TargetTriple);
   appendToGlobalCtors(M, MemProfCtorFunction, Priority);
 
+  createProfileFileNameVar(M);
+
   return true;
 }
 
 void MemProfiler::initializeCallbacks(Module &M) {
   IRBuilder<> IRB(*C);
 
   for (size_t AccessIsWrite = 0; AccessIsWrite <= 1; AccessIsWrite++) {
     const std::string TypeStr = AccessIsWrite ? "store" : "load";

llvm/test/Instrumentation/HeapProfiler/filename.ll

+; 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