[clang] [llvm] [memprof] Add memprof options as a clang frontend flag (PR #128615)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Ellis Hoag (ellishg)

<details>
<summary>Changes</summary>

Add the clang frontend flag `-fmemory-profile-runtime-default-options` to support the `-memprof-runtime-default-options` LLVM flag introduced in https://github.com/llvm/llvm-project/pull/118874.

This enables us to easily pass the `-exported_symbol ___memprof_default_options_str` flag to the linker on Darwin, which is required because `__memprof_default_options_str` uses `WeakAnyLinkage` on Darwin.

https://github.com/ellishg/llvm-project/blob/fa0202169af23419c4bcbf66eabd1beb6b6e8e34/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp#L573-L576

Also, do not emit the `__memprof_default_options_str` symbol when `-memprof-runtime-default-options` is not supplied.

---
Full diff: https://github.com/llvm/llvm-project/pull/128615.diff


6 Files Affected:

- (modified) clang/include/clang/Driver/Options.td (+9) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+7) 
- (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+25-21) 
- (modified) clang/test/Driver/fmemprof.cpp (+7) 
- (modified) llvm/lib/Transforms/Instrumentation/MemProfiler.cpp (+2) 
- (modified) llvm/test/Instrumentation/HeapProfiler/memprof-options.ll (+5-4) 


``````````diff
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e521cbf678d93..2184db6e316b8 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2288,6 +2288,15 @@ def fmemory_profile_use_EQ : Joined<["-"], "fmemory-profile-use=">,
     MetaVarName<"<pathname>">,
     HelpText<"Use memory profile for profile-guided memory optimization">,
     MarshallingInfoString<CodeGenOpts<"MemoryProfileUsePath">>;
+def fmemory_profile_runtime_default_options_EQ
+    : Joined<["-"], "fmemory-profile-runtime-default-options=">,
+      Group<f_Group>,
+      Visibility<[ClangOption, CC1Option, CLOption]>,
+      MetaVarName<"<options>">,
+      HelpText<"Set the default memprof runtime options to <options>">;
+def fmemory_profile_runtime_default_options
+    : Separate<["-"], "fmemory-profile-runtime-default-options">,
+      Alias<fmemory_profile_runtime_default_options_EQ>;
 
 // Begin sanitizer flags. These should all be core options exposed in all driver
 // modes.
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index aa83e3e36124c..7e0983a140af5 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5456,6 +5456,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
           << MemProfUseArg->getAsString(Args) << PGOInstrArg->getAsString(Args);
     MemProfUseArg->render(Args, CmdArgs);
   }
+  StringRef MemprofOptionsStr = Args.getLastArgValue(
+      options::OPT_fmemory_profile_runtime_default_options_EQ);
+  if (!MemprofOptionsStr.empty()) {
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back(Args.MakeArgString("-memprof-runtime-default-options=" +
+                                         MemprofOptionsStr));
+  }
 
   // Embed-bitcode option.
   // Only white-listed flags below are allowed to be embedded.
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 75f126965e0ac..987a88fe4afb9 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -570,6 +570,27 @@ static void renderRemarksOptions(const ArgList &Args, ArgStringList &CmdArgs,
 
 static void AppendPlatformPrefix(SmallString<128> &Path, const llvm::Triple &T);
 
+/// Check if the link command contains a symbol export directive.
+static bool hasExportSymbolDirective(const ArgList &Args) {
+  for (Arg *A : Args) {
+    if (A->getOption().matches(options::OPT_exported__symbols__list))
+      return true;
+    if (!A->getOption().matches(options::OPT_Wl_COMMA) &&
+        !A->getOption().matches(options::OPT_Xlinker))
+      continue;
+    if (A->containsValue("-exported_symbols_list") ||
+        A->containsValue("-exported_symbol"))
+      return true;
+  }
+  return false;
+}
+
+/// Add an export directive for \p Symbol to the link command.
+static void addExportedSymbol(ArgStringList &CmdArgs, const char *Symbol) {
+  CmdArgs.push_back("-exported_symbol");
+  CmdArgs.push_back(Symbol);
+}
+
 void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                   const InputInfo &Output,
                                   const InputInfoList &Inputs,
@@ -734,6 +755,10 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   getMachOToolChain().addProfileRTLibs(Args, CmdArgs);
 
+  if (Args.hasArg(options::OPT_fmemory_profile_runtime_default_options_EQ))
+    if (hasExportSymbolDirective(Args))
+      addExportedSymbol(CmdArgs, "___memprof_default_options_str");
+
   StringRef Parallelism = getLTOParallelism(Args, getToolChain().getDriver());
   if (!Parallelism.empty()) {
     CmdArgs.push_back("-mllvm");
@@ -1433,27 +1458,6 @@ StringRef Darwin::getOSLibraryNameSuffix(bool IgnoreSim) const {
   llvm_unreachable("Unsupported platform");
 }
 
-/// Check if the link command contains a symbol export directive.
-static bool hasExportSymbolDirective(const ArgList &Args) {
-  for (Arg *A : Args) {
-    if (A->getOption().matches(options::OPT_exported__symbols__list))
-      return true;
-    if (!A->getOption().matches(options::OPT_Wl_COMMA) &&
-        !A->getOption().matches(options::OPT_Xlinker))
-      continue;
-    if (A->containsValue("-exported_symbols_list") ||
-        A->containsValue("-exported_symbol"))
-      return true;
-  }
-  return false;
-}
-
-/// Add an export directive for \p Symbol to the link command.
-static void addExportedSymbol(ArgStringList &CmdArgs, const char *Symbol) {
-  CmdArgs.push_back("-exported_symbol");
-  CmdArgs.push_back(Symbol);
-}
-
 /// Add a sectalign directive for \p Segment and \p Section to the maximum
 /// expected page size for Darwin.
 ///
diff --git a/clang/test/Driver/fmemprof.cpp b/clang/test/Driver/fmemprof.cpp
index 5165c4452fd57..4cba8d67ebf74 100644
--- a/clang/test/Driver/fmemprof.cpp
+++ b/clang/test/Driver/fmemprof.cpp
@@ -17,3 +17,10 @@
 
 // RUN: not %clangxx --target=x86_64-linux-gnu -fprofile-generate -fmemory-profile-use=foo %s -### 2>&1 | FileCheck %s --check-prefix=CONFLICTWITHPGOINSTR
 // CONFLICTWITHPGOINSTR: error: invalid argument '-fmemory-profile-use=foo' not allowed with '-fprofile-generate'
+
+// RUN: %clangxx -target arm64-apple-ios -fmemory-profile -fmemory-profile-runtime-default-options="verbose=1" %s -### 2>&1 | FileCheck %s --check-prefix=OPTS
+// RUN: %clangxx -target arm64-apple-ios -fmemory-profile -fmemory-profile-runtime-default-options "verbose=1" %s -### 2>&1 | FileCheck %s --check-prefix=OPTS
+// RUN: %clangxx -target arm64-apple-ios -fmemory-profile -fmemory-profile-runtime-default-options="verbose=1" -exported_symbols_list /dev/null %s -### 2>&1 | FileCheck %s --check-prefixes=OPTS,OPTS-EXPORT
+// RUN: %clangxx -target arm64-apple-ios -fmemory-profile -fmemory-profile-runtime-default-options "verbose=1" -exported_symbols_list /dev/null %s -### 2>&1 | FileCheck %s --check-prefixes=OPTS,OPTS-EXPORT
+// OPTS: "-mllvm" "-memprof-runtime-default-options=verbose=1"
+// OPTS-EXPORT: "-exported_symbol" "___memprof_default_options_str"
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 7d8bc3aa4c589..8bfc2ed241a5e 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -566,6 +566,8 @@ void createMemprofHistogramFlagVar(Module &M) {
 }
 
 void createMemprofDefaultOptionsVar(Module &M) {
+  if (!MemprofRuntimeDefaultOptions.getNumOccurrences())
+    return;
   Constant *OptionsConst = ConstantDataArray::getString(
       M.getContext(), MemprofRuntimeDefaultOptions, /*AddNull=*/true);
   GlobalVariable *OptionsVar =
diff --git a/llvm/test/Instrumentation/HeapProfiler/memprof-options.ll b/llvm/test/Instrumentation/HeapProfiler/memprof-options.ll
index 8e82d524952dd..723f26c17d441 100644
--- a/llvm/test/Instrumentation/HeapProfiler/memprof-options.ll
+++ b/llvm/test/Instrumentation/HeapProfiler/memprof-options.ll
@@ -1,11 +1,12 @@
-; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes='function(memprof),memprof-module' -S | FileCheck %s --check-prefixes=CHECK,EMPTY
-; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes='function(memprof),memprof-module' -S -memprof-runtime-default-options="verbose=1" | FileCheck %s --check-prefixes=CHECK,VERBOSE
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes='function(memprof),memprof-module' -S | FileCheck %s --check-prefix=EMPTY
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes='function(memprof),memprof-module' -S -memprof-runtime-default-options="verbose=1" | FileCheck %s
 
 define i32 @main() {
 entry:
   ret i32 0
 }
 
+; EMPTY-NOT: memprof_default_options_str
+
 ; CHECK: $__memprof_default_options_str = comdat any
-; EMPTY: @__memprof_default_options_str = constant [1 x i8] zeroinitializer, comdat
-; VERBOSE: @__memprof_default_options_str = constant [10 x i8] c"verbose=1\00", comdat
+; CHECK: @__memprof_default_options_str = constant [10 x i8] c"verbose=1\00", comdat

``````````

</details>


https://github.com/llvm/llvm-project/pull/128615