Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][flang][mlir] Support -frecord-command-line option #102975

Merged
merged 6 commits into from
Sep 20, 2024

Conversation

tarunprabhu
Copy link
Contributor

@tarunprabhu tarunprabhu commented Aug 12, 2024

Add support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang.


Notes for reviewers: In #98517, the tune-cpu argument was added to the constructor of the lowering bridge object in flang. This PR would have required adding another option to it. It is possible more such arguments may be added in the future. To avoid having to add new arguments each time, the TargetOptions and CodeGenOptions objects from the frontend are now passed directly through to the lowering bridge constructor.

This required a change to bbc where empty TargetOptions and CodeGenOptions objects had to be created.

If others have better suggestions for how to handle this, I am happy to make changes.

[EDIT: Fix typo]

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' mlir:llvm flang:driver mlir flang Flang issues not falling into any other category flang:fir-hlfir labels Aug 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2024

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-mlir

Author: Tarun Prabhu (tarunprabhu)

Changes

Add support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang.


Notes for reviewers: In #98517, the tune-cpu argument was added to the constructor of the lowering bridge object in flang. This PR would have required adding another option to it. It is possible more such arguments may be added in the future. To avoid having to add new arguments each time, the constructor the TargetOptions and CodeGenOptions objects from the frontend are now passed directly through to the lowering bridge.

This required a change to bbc where empty TargetOptions and CodeGenOptions objects had to be created.

If others have better suggestions for how to handle this, I am happy to make changes.


Patch is 28.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102975.diff

24 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+7-5)
  • (modified) clang/lib/Driver/CMakeLists.txt (+1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+5-43)
  • (added) clang/lib/Driver/ToolChains/CommonUtils.cpp (+76)
  • (added) clang/lib/Driver/ToolChains/CommonUtils.h (+44)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+15)
  • (modified) flang/include/flang/Frontend/CodeGenOptions.h (+3)
  • (modified) flang/include/flang/Lower/Bridge.h (+9-3)
  • (modified) flang/include/flang/Optimizer/Dialect/Support/FIRContext.h (+6)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+6)
  • (modified) flang/lib/Frontend/FrontendActions.cpp (+1-1)
  • (modified) flang/lib/Lower/Bridge.cpp (+6-2)
  • (modified) flang/lib/Optimizer/Dialect/Support/FIRContext.cpp (+16)
  • (added) flang/test/Driver/frecord-command-line.f90 (+11)
  • (added) flang/test/Lower/record-command-line.f90 (+9)
  • (modified) flang/tools/bbc/CMakeLists.txt (+1)
  • (modified) flang/tools/bbc/bbc.cpp (+5-2)
  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td (+1)
  • (modified) mlir/include/mlir/Target/LLVMIR/ModuleImport.h (+4)
  • (modified) mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h (+3)
  • (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+19)
  • (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+17)
  • (added) mlir/test/Target/LLVMIR/Import/commandline.ll (+6)
  • (added) mlir/test/Target/LLVMIR/commandline.mlir (+6)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0b38139bd27972..670e3658b6e85c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1980,9 +1980,11 @@ within a command line are combined with spaces; spaces and backslashes within an
 argument are escaped with backslashes. This format differs from the format of
 the equivalent section produced by GCC with the -frecord-gcc-switches flag.
 This option is currently only supported on ELF targets.}]>,
-  Group<f_clang_Group>;
+  Group<f_clang_Group>,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def fno_record_command_line : Flag<["-"], "fno-record-command-line">,
-  Group<f_clang_Group>;
+  Group<f_clang_Group>,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def : Flag<["-"], "frecord-gcc-switches">, Alias<frecord_command_line>;
 def : Flag<["-"], "fno-record-gcc-switches">, Alias<fno_record_command_line>;
 def fcommon : Flag<["-"], "fcommon">, Group<f_Group>,
@@ -7074,6 +7076,9 @@ def mrelocation_model : Separate<["-"], "mrelocation-model">,
   NormalizedValues<["Static", "PIC_", "ROPI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
   MarshallingInfoEnum<CodeGenOpts<"RelocationModel">, "PIC_">;
 def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">;
+def record_command_line : Separate<["-"], "record-command-line">,
+  HelpText<"The string to embed in the .LLVM.command.line section.">,
+  MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
 
 } // let Visibility = [CC1Option, CC1AsOption, FC1Option]
 
@@ -7094,9 +7099,6 @@ def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">,
 def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
   HelpText<"The string to embed in the Dwarf debug flags record.">,
   MarshallingInfoString<CodeGenOpts<"DwarfDebugFlags">>;
-def record_command_line : Separate<["-"], "record-command-line">,
-  HelpText<"The string to embed in the .LLVM.command.line section.">,
-  MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
 def compress_debug_sections_EQ : Joined<["-", "--"], "compress-debug-sections=">,
     HelpText<"DWARF debug sections compression type">, Values<"none,zlib,zstd">,
     NormalizedValuesScope<"llvm::DebugCompressionType">, NormalizedValues<["None", "Zlib", "Zstd"]>,
diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index 32a4378ab499fa..0b92077384e9bc 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -48,6 +48,7 @@ add_clang_library(clangDriver
   ToolChains/BareMetal.cpp
   ToolChains/Clang.cpp
   ToolChains/CommonArgs.cpp
+  ToolChains/CommonUtils.cpp
   ToolChains/CrossWindows.cpp
   ToolChains/CSKYToolChain.cpp
   ToolChains/Cuda.cpp
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c698d38b80e578..e98093c855548d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -21,6 +21,7 @@
 #include "Arch/VE.h"
 #include "Arch/X86.h"
 #include "CommonArgs.h"
+#include "CommonUtils.h"
 #include "Hexagon.h"
 #include "MSP430.h"
 #include "PS4CPU.h"
@@ -94,24 +95,6 @@ static void CheckCodeGenerationOptions(const Driver &D, const ArgList &Args) {
                                                       << "-static";
 }
 
-// Add backslashes to escape spaces and other backslashes.
-// This is used for the space-separated argument list specified with
-// the -dwarf-debug-flags option.
-static void EscapeSpacesAndBackslashes(const char *Arg,
-                                       SmallVectorImpl<char> &Res) {
-  for (; *Arg; ++Arg) {
-    switch (*Arg) {
-    default:
-      break;
-    case ' ':
-    case '\\':
-      Res.push_back('\\');
-      break;
-    }
-    Res.push_back(*Arg);
-  }
-}
-
 /// Apply \a Work on the current tool chain \a RegularToolChain and any other
 /// offloading tool chain that is associated with the current action \a JA.
 static void
@@ -7662,31 +7645,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   // Also record command line arguments into the debug info if
   // -grecord-gcc-switches options is set on.
   // By default, -gno-record-gcc-switches is set on and no recording.
-  auto GRecordSwitches =
-      Args.hasFlag(options::OPT_grecord_command_line,
-                   options::OPT_gno_record_command_line, false);
-  auto FRecordSwitches =
-      Args.hasFlag(options::OPT_frecord_command_line,
-                   options::OPT_fno_record_command_line, false);
-  if (FRecordSwitches && !Triple.isOSBinFormatELF() &&
-      !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
-    D.Diag(diag::err_drv_unsupported_opt_for_target)
-        << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
-        << TripleStr;
-  if (TC.UseDwarfDebugFlags() || GRecordSwitches || FRecordSwitches) {
-    ArgStringList OriginalArgs;
-    for (const auto &Arg : Args)
-      Arg->render(Args, OriginalArgs);
-
-    SmallString<256> Flags;
-    EscapeSpacesAndBackslashes(Exec, Flags);
-    for (const char *OriginalArg : OriginalArgs) {
-      SmallString<128> EscapedArg;
-      EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
-      Flags += " ";
-      Flags += EscapedArg;
-    }
-    auto FlagsArgString = Args.MakeArgString(Flags);
+  auto GRecordSwitches = false;
+  auto FRecordSwitches = false;
+  if (ShouldRecordCommandLine(TC, Args, FRecordSwitches, GRecordSwitches)) {
+    auto FlagsArgString = RenderEscapedCommandLine(TC, Args);
     if (TC.UseDwarfDebugFlags() || GRecordSwitches) {
       CmdArgs.push_back("-dwarf-debug-flags");
       CmdArgs.push_back(FlagsArgString);
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.cpp b/clang/lib/Driver/ToolChains/CommonUtils.cpp
new file mode 100644
index 00000000000000..20d76990517a43
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.cpp
@@ -0,0 +1,76 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CommonUtils.h"
+#include "clang/Driver/Driver.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::driver;
+
+namespace clang {
+
+void EscapeSpacesAndBackslashes(const char *Arg,
+                                llvm::SmallVectorImpl<char> &Res) {
+  for (; *Arg; ++Arg) {
+    switch (*Arg) {
+    default:
+      break;
+    case ' ':
+    case '\\':
+      Res.push_back('\\');
+      break;
+    }
+    Res.push_back(*Arg);
+  }
+}
+
+const char *RenderEscapedCommandLine(const ToolChain &TC,
+                                     const llvm::opt::ArgList &Args) {
+  const Driver &D = TC.getDriver();
+  const char *Exec = D.getClangProgramPath();
+
+  llvm::opt::ArgStringList OriginalArgs;
+  for (const auto &Arg : Args)
+    Arg->render(Args, OriginalArgs);
+
+  llvm::SmallString<256> Flags;
+  EscapeSpacesAndBackslashes(Exec, Flags);
+  for (const char *OriginalArg : OriginalArgs) {
+    llvm::SmallString<128> EscapedArg;
+    EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
+    Flags += " ";
+    Flags += EscapedArg;
+  }
+
+  return Args.MakeArgString(Flags);
+}
+
+bool ShouldRecordCommandLine(const ToolChain &TC,
+                             const llvm::opt::ArgList &Args,
+                             bool &FRecordCommandLine,
+                             bool &GRecordCommandLine) {
+  const Driver &D = TC.getDriver();
+  const llvm::Triple &Triple = TC.getEffectiveTriple();
+  const std::string &TripleStr = Triple.getTriple();
+
+  FRecordCommandLine =
+      Args.hasFlag(options::OPT_frecord_command_line,
+                   options::OPT_fno_record_command_line, false);
+  GRecordCommandLine =
+      Args.hasFlag(options::OPT_grecord_command_line,
+                   options::OPT_gno_record_command_line, false);
+  if (FRecordCommandLine && !Triple.isOSBinFormatELF() &&
+      !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
+    D.Diag(diag::err_drv_unsupported_opt_for_target)
+        << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
+        << TripleStr;
+
+  return FRecordCommandLine || TC.UseDwarfDebugFlags() || GRecordCommandLine;
+}
+
+} // namespace clang
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.h b/clang/lib/Driver/ToolChains/CommonUtils.h
new file mode 100644
index 00000000000000..1ae1c54cf58db4
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.h
@@ -0,0 +1,44 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+
+#include "clang/Driver/ToolChain.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/ArgList.h"
+
+namespace clang {
+
+// Add backslashes to escape spaces and other backslashes.
+// This is used for the space-separated argument list specified with
+// the -dwarf-debug-flags option.
+void EscapeSpacesAndBackslashes(const char *Arg,
+                                llvm::SmallVectorImpl<char> &Res);
+
+/// Join the args in the given ArgList, escape spaces and backslashes and
+/// return the joined string. This is used when saving the command line as a
+/// result of using either the -frecord-command-line or -grecord-command-line
+/// options. The lifetime of the returned c-string will match that of the Args
+/// argument.
+const char *RenderEscapedCommandLine(const clang::driver::ToolChain &TC,
+                                     const llvm::opt::ArgList &Args);
+
+/// Check if the command line should be recorded in the object file. This is
+/// done if either -frecord-command-line or -grecord-command-line options have
+/// been passed. This also does some error checking since -frecord-command-line
+/// is currently only supported on ELF platforms. The last two boolean
+/// arguments are out parameters and will be set depending on the command
+/// line options that were passed.
+bool ShouldRecordCommandLine(const clang::driver::ToolChain &TC,
+                             const llvm::opt::ArgList &Args,
+                             bool &FRecordCommandLine,
+                             bool &GRecordCommandLine);
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6ce79d27e98c48..2ef14e306cf324 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -9,6 +9,7 @@
 #include "Flang.h"
 #include "Arch/RISCV.h"
 #include "CommonArgs.h"
+#include "CommonUtils.h"
 
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Driver/Options.h"
@@ -885,6 +886,20 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
 
   addDashXForInput(Args, Input, CmdArgs);
 
+  bool FRecordCmdLine = false;
+  bool GRecordCmdLine = false;
+  if (ShouldRecordCommandLine(TC, Args, FRecordCmdLine, GRecordCmdLine)) {
+    const char *CmdLine = RenderEscapedCommandLine(TC, Args);
+    if (FRecordCmdLine) {
+      CmdArgs.push_back("-record-command-line");
+      CmdArgs.push_back(CmdLine);
+    }
+    if (TC.UseDwarfDebugFlags() || GRecordCmdLine) {
+      CmdArgs.push_back("-dwarf-debug-flags");
+      CmdArgs.push_back(CmdLine);
+    }
+  }
+
   CmdArgs.push_back(Input.getFilename());
 
   // TODO: Replace flang-new with flang once the new driver replaces the
diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h
index ac7fcbcba4f747..f19943335737b9 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.h
+++ b/flang/include/flang/Frontend/CodeGenOptions.h
@@ -63,6 +63,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// The directory where temp files are stored if specified by -save-temps
   std::optional<std::string> SaveTempsDir;
 
+  /// The string containing the commandline for the llvm.commandline metadata.
+  std::optional<std::string> RecordCommandLine;
+
   /// The name of the file to which the backend should save YAML optimization
   /// records.
   std::string OptRecordFile;
diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h
index 4379ed512cdf0a..8ea5ed52e28218 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -14,6 +14,8 @@
 #define FORTRAN_LOWER_BRIDGE_H
 
 #include "flang/Common/Fortran.h"
+#include "flang/Frontend/CodeGenOptions.h"
+#include "flang/Frontend/TargetOptions.h"
 #include "flang/Lower/AbstractConverter.h"
 #include "flang/Lower/EnvironmentDefault.h"
 #include "flang/Lower/LoweringOptions.h"
@@ -65,11 +67,13 @@ class LoweringBridge {
          const Fortran::lower::LoweringOptions &loweringOptions,
          const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
          const Fortran::common::LanguageFeatureControl &languageFeatures,
-         const llvm::TargetMachine &targetMachine, llvm::StringRef tuneCPU) {
+         const llvm::TargetMachine &targetMachine,
+         const Fortran::frontend::TargetOptions &targetOptions,
+         const Fortran::frontend::CodeGenOptions &codeGenOptions) {
     return LoweringBridge(ctx, semanticsContext, defaultKinds, intrinsics,
                           targetCharacteristics, allCooked, triple, kindMap,
                           loweringOptions, envDefaults, languageFeatures,
-                          targetMachine, tuneCPU);
+                          targetMachine, targetOptions, codeGenOptions);
   }
 
   //===--------------------------------------------------------------------===//
@@ -148,7 +152,9 @@ class LoweringBridge {
       const Fortran::lower::LoweringOptions &loweringOptions,
       const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
       const Fortran::common::LanguageFeatureControl &languageFeatures,
-      const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU);
+      const llvm::TargetMachine &targetMachine,
+      const Fortran::frontend::TargetOptions &targetOptions,
+      const Fortran::frontend::CodeGenOptions &codeGenOptions);
   LoweringBridge() = delete;
   LoweringBridge(const LoweringBridge &) = delete;
 
diff --git a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
index e45011c8e02a33..2df14f83c11e17 100644
--- a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
+++ b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
@@ -77,6 +77,12 @@ void setIdent(mlir::ModuleOp mod, llvm::StringRef ident);
 /// Get the compiler identifier from the Module.
 llvm::StringRef getIdent(mlir::ModuleOp mod);
 
+/// Set the command line used in this invocation.
+void setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine);
+
+/// Get the command line used in this invocation.
+llvm::StringRef getCommandline(mlir::ModuleOp mod);
+
 /// Helper for determining the target from the host, etc. Tools may use this
 /// function to provide a consistent interpretation of the `--target=<string>`
 /// command-line option.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1d73397d330178..4e8fd44f526a5c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -348,6 +348,12 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
   if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
     opts.SaveTempsDir = a->getValue();
 
+  // -record-command-line option.
+  if (const llvm::opt::Arg *a =
+          args.getLastArg(clang::driver::options::OPT_record_command_line)) {
+    opts.RecordCommandLine = a->getValue();
+  }
+
   // -mlink-builtin-bitcode
   for (auto *a :
        args.filtered(clang::driver::options::OPT_mlink_builtin_bitcode))
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 5c86bd947ce73f..909e5909938d87 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -298,7 +298,7 @@ bool CodeGenAction::beginSourceFileAction() {
       kindMap, ci.getInvocation().getLoweringOpts(),
       ci.getInvocation().getFrontendOpts().envDefaults,
       ci.getInvocation().getFrontendOpts().features, targetMachine,
-      ci.getInvocation().getTargetOpts().cpuToTuneFor);
+      ci.getInvocation().getTargetOpts(), ci.getInvocation().getCodeGenOpts());
 
   // Fetch module from lb, so we can set
   mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ccbb481f472d81..2b42c3e355fbcc 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -6065,7 +6065,9 @@ Fortran::lower::LoweringBridge::LoweringBridge(
     const Fortran::lower::LoweringOptions &loweringOptions,
     const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
     const Fortran::common::LanguageFeatureControl &languageFeatures,
-    const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU)
+    const llvm::TargetMachine &targetMachine,
+    const Fortran::frontend::TargetOptions &targetOpts,
+    const Fortran::frontend::CodeGenOptions &cgOpts)
     : semanticsContext{semanticsContext}, defaultKinds{defaultKinds},
       intrinsics{intrinsics}, targetCharacteristics{targetCharacteristics},
       cooked{&cooked}, context{context}, kindMap{kindMap},
@@ -6122,11 +6124,13 @@ Fortran::lower::LoweringBridge::LoweringBridge(
   fir::setTargetTriple(*module.get(), triple);
   fir::setKindMapping(*module.get(), kindMap);
   fir::setTargetCPU(*module.get(), targetMachine.getTargetCPU());
-  fir::setTuneCPU(*module.get(), tuneCPU);
+  fir::setTuneCPU(*module.get(), targetOpts.cpuToTuneFor);
   fir::setTargetFeatures(*module.get(), targetMachine.getTargetFeatureString());
   fir::support::setMLIRDataLayout(*module.get(),
                                   targetMachine.createDataLayout());
   fir::setIdent(*module.get(), Fortran::common::getFlangFullVersion());
+  if (cgOpts.RecordCommandLine)
+    fir::setCommandline(*module.get(), *cgOpts.RecordCommandLine);
 }
 
 void Fortran::lower::genCleanUpInRegionIfAny(
diff --git a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
index 5bd8e2d4336361..01c0be66d1ecc3 100644
--- a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
+++ b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
@@ -130,6 +130,22 @@ llvm::StringRef fir::getIdent(mlir::ModuleOp mod) {
   return {};
 }
 
+void fir::setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine) {
+  if (cmdLine.empty())
+    return;
+
+  mlir::MLIRContext *ctx = mod.getContext();
+  mod->setAttr(mlir::LLVM::LLVMDialect::getCommandlineAttrName(),
+               mlir::StringAttr::get(ctx, cmdLine));
+}
+
+llvm::StringRef fir::getCommandline(mlir::ModuleOp mod) {
+  if (auto attr = mod->getAttrOfType<mlir::StringAttr>(
+          mlir::LLVM::LLVMDialect::getCommandlineAttrName()))
+    return attr;
+  return {};
+}
+
 std::string fir::determineTargetTriple(llvm::StringRef triple) {
   // Treat "" or "default" as stand-ins for the default machine.
   if (triple.empty() || triple == "default")
diff --git a/flang/test/Driver/frecord-command-line.f90 b/flang/test/Driver/frecord-command-line.f90
new file mode 100644
index 00000000000000..3dc65587ee2563
--- /dev/null
+++ b/flang/test/Driver/frecord-command-line.f90
@@ -0,0 +1,11 @@
+! This only checks that the command line is correctly passed on to the
+! -record-command-line option FC1 option and that the latter does not complain
+! about anything. The correct lowering to a module attribute and beyond will
+! be checked in other tests.
+!
+! RUN: %flang -### -frecord-command-line %s 2>&1 | FileCheck --check-prefix=CHECK-RECORD ...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2024

@llvm/pr-subscribers-clang

Author: Tarun Prabhu (tarunprabhu)

Changes

Add support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang.


Notes for reviewers: In #98517, the tune-cpu argument was added to the constructor of the lowering bridge object in flang. This PR would have required adding another option to it. It is possible more such arguments may be added in the future. To avoid having to add new arguments each time, the constructor the TargetOptions and CodeGenOptions objects from the frontend are now passed directly through to the lowering bridge.

This required a change to bbc where empty TargetOptions and CodeGenOptions objects had to be created.

If others have better suggestions for how to handle this, I am happy to make changes.


Patch is 28.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102975.diff

24 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+7-5)
  • (modified) clang/lib/Driver/CMakeLists.txt (+1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+5-43)
  • (added) clang/lib/Driver/ToolChains/CommonUtils.cpp (+76)
  • (added) clang/lib/Driver/ToolChains/CommonUtils.h (+44)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+15)
  • (modified) flang/include/flang/Frontend/CodeGenOptions.h (+3)
  • (modified) flang/include/flang/Lower/Bridge.h (+9-3)
  • (modified) flang/include/flang/Optimizer/Dialect/Support/FIRContext.h (+6)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+6)
  • (modified) flang/lib/Frontend/FrontendActions.cpp (+1-1)
  • (modified) flang/lib/Lower/Bridge.cpp (+6-2)
  • (modified) flang/lib/Optimizer/Dialect/Support/FIRContext.cpp (+16)
  • (added) flang/test/Driver/frecord-command-line.f90 (+11)
  • (added) flang/test/Lower/record-command-line.f90 (+9)
  • (modified) flang/tools/bbc/CMakeLists.txt (+1)
  • (modified) flang/tools/bbc/bbc.cpp (+5-2)
  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td (+1)
  • (modified) mlir/include/mlir/Target/LLVMIR/ModuleImport.h (+4)
  • (modified) mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h (+3)
  • (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+19)
  • (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+17)
  • (added) mlir/test/Target/LLVMIR/Import/commandline.ll (+6)
  • (added) mlir/test/Target/LLVMIR/commandline.mlir (+6)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0b38139bd27972..670e3658b6e85c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1980,9 +1980,11 @@ within a command line are combined with spaces; spaces and backslashes within an
 argument are escaped with backslashes. This format differs from the format of
 the equivalent section produced by GCC with the -frecord-gcc-switches flag.
 This option is currently only supported on ELF targets.}]>,
-  Group<f_clang_Group>;
+  Group<f_clang_Group>,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def fno_record_command_line : Flag<["-"], "fno-record-command-line">,
-  Group<f_clang_Group>;
+  Group<f_clang_Group>,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def : Flag<["-"], "frecord-gcc-switches">, Alias<frecord_command_line>;
 def : Flag<["-"], "fno-record-gcc-switches">, Alias<fno_record_command_line>;
 def fcommon : Flag<["-"], "fcommon">, Group<f_Group>,
@@ -7074,6 +7076,9 @@ def mrelocation_model : Separate<["-"], "mrelocation-model">,
   NormalizedValues<["Static", "PIC_", "ROPI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
   MarshallingInfoEnum<CodeGenOpts<"RelocationModel">, "PIC_">;
 def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">;
+def record_command_line : Separate<["-"], "record-command-line">,
+  HelpText<"The string to embed in the .LLVM.command.line section.">,
+  MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
 
 } // let Visibility = [CC1Option, CC1AsOption, FC1Option]
 
@@ -7094,9 +7099,6 @@ def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">,
 def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
   HelpText<"The string to embed in the Dwarf debug flags record.">,
   MarshallingInfoString<CodeGenOpts<"DwarfDebugFlags">>;
-def record_command_line : Separate<["-"], "record-command-line">,
-  HelpText<"The string to embed in the .LLVM.command.line section.">,
-  MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
 def compress_debug_sections_EQ : Joined<["-", "--"], "compress-debug-sections=">,
     HelpText<"DWARF debug sections compression type">, Values<"none,zlib,zstd">,
     NormalizedValuesScope<"llvm::DebugCompressionType">, NormalizedValues<["None", "Zlib", "Zstd"]>,
diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index 32a4378ab499fa..0b92077384e9bc 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -48,6 +48,7 @@ add_clang_library(clangDriver
   ToolChains/BareMetal.cpp
   ToolChains/Clang.cpp
   ToolChains/CommonArgs.cpp
+  ToolChains/CommonUtils.cpp
   ToolChains/CrossWindows.cpp
   ToolChains/CSKYToolChain.cpp
   ToolChains/Cuda.cpp
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c698d38b80e578..e98093c855548d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -21,6 +21,7 @@
 #include "Arch/VE.h"
 #include "Arch/X86.h"
 #include "CommonArgs.h"
+#include "CommonUtils.h"
 #include "Hexagon.h"
 #include "MSP430.h"
 #include "PS4CPU.h"
@@ -94,24 +95,6 @@ static void CheckCodeGenerationOptions(const Driver &D, const ArgList &Args) {
                                                       << "-static";
 }
 
-// Add backslashes to escape spaces and other backslashes.
-// This is used for the space-separated argument list specified with
-// the -dwarf-debug-flags option.
-static void EscapeSpacesAndBackslashes(const char *Arg,
-                                       SmallVectorImpl<char> &Res) {
-  for (; *Arg; ++Arg) {
-    switch (*Arg) {
-    default:
-      break;
-    case ' ':
-    case '\\':
-      Res.push_back('\\');
-      break;
-    }
-    Res.push_back(*Arg);
-  }
-}
-
 /// Apply \a Work on the current tool chain \a RegularToolChain and any other
 /// offloading tool chain that is associated with the current action \a JA.
 static void
@@ -7662,31 +7645,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   // Also record command line arguments into the debug info if
   // -grecord-gcc-switches options is set on.
   // By default, -gno-record-gcc-switches is set on and no recording.
-  auto GRecordSwitches =
-      Args.hasFlag(options::OPT_grecord_command_line,
-                   options::OPT_gno_record_command_line, false);
-  auto FRecordSwitches =
-      Args.hasFlag(options::OPT_frecord_command_line,
-                   options::OPT_fno_record_command_line, false);
-  if (FRecordSwitches && !Triple.isOSBinFormatELF() &&
-      !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
-    D.Diag(diag::err_drv_unsupported_opt_for_target)
-        << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
-        << TripleStr;
-  if (TC.UseDwarfDebugFlags() || GRecordSwitches || FRecordSwitches) {
-    ArgStringList OriginalArgs;
-    for (const auto &Arg : Args)
-      Arg->render(Args, OriginalArgs);
-
-    SmallString<256> Flags;
-    EscapeSpacesAndBackslashes(Exec, Flags);
-    for (const char *OriginalArg : OriginalArgs) {
-      SmallString<128> EscapedArg;
-      EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
-      Flags += " ";
-      Flags += EscapedArg;
-    }
-    auto FlagsArgString = Args.MakeArgString(Flags);
+  auto GRecordSwitches = false;
+  auto FRecordSwitches = false;
+  if (ShouldRecordCommandLine(TC, Args, FRecordSwitches, GRecordSwitches)) {
+    auto FlagsArgString = RenderEscapedCommandLine(TC, Args);
     if (TC.UseDwarfDebugFlags() || GRecordSwitches) {
       CmdArgs.push_back("-dwarf-debug-flags");
       CmdArgs.push_back(FlagsArgString);
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.cpp b/clang/lib/Driver/ToolChains/CommonUtils.cpp
new file mode 100644
index 00000000000000..20d76990517a43
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.cpp
@@ -0,0 +1,76 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CommonUtils.h"
+#include "clang/Driver/Driver.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::driver;
+
+namespace clang {
+
+void EscapeSpacesAndBackslashes(const char *Arg,
+                                llvm::SmallVectorImpl<char> &Res) {
+  for (; *Arg; ++Arg) {
+    switch (*Arg) {
+    default:
+      break;
+    case ' ':
+    case '\\':
+      Res.push_back('\\');
+      break;
+    }
+    Res.push_back(*Arg);
+  }
+}
+
+const char *RenderEscapedCommandLine(const ToolChain &TC,
+                                     const llvm::opt::ArgList &Args) {
+  const Driver &D = TC.getDriver();
+  const char *Exec = D.getClangProgramPath();
+
+  llvm::opt::ArgStringList OriginalArgs;
+  for (const auto &Arg : Args)
+    Arg->render(Args, OriginalArgs);
+
+  llvm::SmallString<256> Flags;
+  EscapeSpacesAndBackslashes(Exec, Flags);
+  for (const char *OriginalArg : OriginalArgs) {
+    llvm::SmallString<128> EscapedArg;
+    EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
+    Flags += " ";
+    Flags += EscapedArg;
+  }
+
+  return Args.MakeArgString(Flags);
+}
+
+bool ShouldRecordCommandLine(const ToolChain &TC,
+                             const llvm::opt::ArgList &Args,
+                             bool &FRecordCommandLine,
+                             bool &GRecordCommandLine) {
+  const Driver &D = TC.getDriver();
+  const llvm::Triple &Triple = TC.getEffectiveTriple();
+  const std::string &TripleStr = Triple.getTriple();
+
+  FRecordCommandLine =
+      Args.hasFlag(options::OPT_frecord_command_line,
+                   options::OPT_fno_record_command_line, false);
+  GRecordCommandLine =
+      Args.hasFlag(options::OPT_grecord_command_line,
+                   options::OPT_gno_record_command_line, false);
+  if (FRecordCommandLine && !Triple.isOSBinFormatELF() &&
+      !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
+    D.Diag(diag::err_drv_unsupported_opt_for_target)
+        << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
+        << TripleStr;
+
+  return FRecordCommandLine || TC.UseDwarfDebugFlags() || GRecordCommandLine;
+}
+
+} // namespace clang
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.h b/clang/lib/Driver/ToolChains/CommonUtils.h
new file mode 100644
index 00000000000000..1ae1c54cf58db4
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.h
@@ -0,0 +1,44 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+
+#include "clang/Driver/ToolChain.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/ArgList.h"
+
+namespace clang {
+
+// Add backslashes to escape spaces and other backslashes.
+// This is used for the space-separated argument list specified with
+// the -dwarf-debug-flags option.
+void EscapeSpacesAndBackslashes(const char *Arg,
+                                llvm::SmallVectorImpl<char> &Res);
+
+/// Join the args in the given ArgList, escape spaces and backslashes and
+/// return the joined string. This is used when saving the command line as a
+/// result of using either the -frecord-command-line or -grecord-command-line
+/// options. The lifetime of the returned c-string will match that of the Args
+/// argument.
+const char *RenderEscapedCommandLine(const clang::driver::ToolChain &TC,
+                                     const llvm::opt::ArgList &Args);
+
+/// Check if the command line should be recorded in the object file. This is
+/// done if either -frecord-command-line or -grecord-command-line options have
+/// been passed. This also does some error checking since -frecord-command-line
+/// is currently only supported on ELF platforms. The last two boolean
+/// arguments are out parameters and will be set depending on the command
+/// line options that were passed.
+bool ShouldRecordCommandLine(const clang::driver::ToolChain &TC,
+                             const llvm::opt::ArgList &Args,
+                             bool &FRecordCommandLine,
+                             bool &GRecordCommandLine);
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6ce79d27e98c48..2ef14e306cf324 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -9,6 +9,7 @@
 #include "Flang.h"
 #include "Arch/RISCV.h"
 #include "CommonArgs.h"
+#include "CommonUtils.h"
 
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Driver/Options.h"
@@ -885,6 +886,20 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
 
   addDashXForInput(Args, Input, CmdArgs);
 
+  bool FRecordCmdLine = false;
+  bool GRecordCmdLine = false;
+  if (ShouldRecordCommandLine(TC, Args, FRecordCmdLine, GRecordCmdLine)) {
+    const char *CmdLine = RenderEscapedCommandLine(TC, Args);
+    if (FRecordCmdLine) {
+      CmdArgs.push_back("-record-command-line");
+      CmdArgs.push_back(CmdLine);
+    }
+    if (TC.UseDwarfDebugFlags() || GRecordCmdLine) {
+      CmdArgs.push_back("-dwarf-debug-flags");
+      CmdArgs.push_back(CmdLine);
+    }
+  }
+
   CmdArgs.push_back(Input.getFilename());
 
   // TODO: Replace flang-new with flang once the new driver replaces the
diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h
index ac7fcbcba4f747..f19943335737b9 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.h
+++ b/flang/include/flang/Frontend/CodeGenOptions.h
@@ -63,6 +63,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// The directory where temp files are stored if specified by -save-temps
   std::optional<std::string> SaveTempsDir;
 
+  /// The string containing the commandline for the llvm.commandline metadata.
+  std::optional<std::string> RecordCommandLine;
+
   /// The name of the file to which the backend should save YAML optimization
   /// records.
   std::string OptRecordFile;
diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h
index 4379ed512cdf0a..8ea5ed52e28218 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -14,6 +14,8 @@
 #define FORTRAN_LOWER_BRIDGE_H
 
 #include "flang/Common/Fortran.h"
+#include "flang/Frontend/CodeGenOptions.h"
+#include "flang/Frontend/TargetOptions.h"
 #include "flang/Lower/AbstractConverter.h"
 #include "flang/Lower/EnvironmentDefault.h"
 #include "flang/Lower/LoweringOptions.h"
@@ -65,11 +67,13 @@ class LoweringBridge {
          const Fortran::lower::LoweringOptions &loweringOptions,
          const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
          const Fortran::common::LanguageFeatureControl &languageFeatures,
-         const llvm::TargetMachine &targetMachine, llvm::StringRef tuneCPU) {
+         const llvm::TargetMachine &targetMachine,
+         const Fortran::frontend::TargetOptions &targetOptions,
+         const Fortran::frontend::CodeGenOptions &codeGenOptions) {
     return LoweringBridge(ctx, semanticsContext, defaultKinds, intrinsics,
                           targetCharacteristics, allCooked, triple, kindMap,
                           loweringOptions, envDefaults, languageFeatures,
-                          targetMachine, tuneCPU);
+                          targetMachine, targetOptions, codeGenOptions);
   }
 
   //===--------------------------------------------------------------------===//
@@ -148,7 +152,9 @@ class LoweringBridge {
       const Fortran::lower::LoweringOptions &loweringOptions,
       const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
       const Fortran::common::LanguageFeatureControl &languageFeatures,
-      const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU);
+      const llvm::TargetMachine &targetMachine,
+      const Fortran::frontend::TargetOptions &targetOptions,
+      const Fortran::frontend::CodeGenOptions &codeGenOptions);
   LoweringBridge() = delete;
   LoweringBridge(const LoweringBridge &) = delete;
 
diff --git a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
index e45011c8e02a33..2df14f83c11e17 100644
--- a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
+++ b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
@@ -77,6 +77,12 @@ void setIdent(mlir::ModuleOp mod, llvm::StringRef ident);
 /// Get the compiler identifier from the Module.
 llvm::StringRef getIdent(mlir::ModuleOp mod);
 
+/// Set the command line used in this invocation.
+void setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine);
+
+/// Get the command line used in this invocation.
+llvm::StringRef getCommandline(mlir::ModuleOp mod);
+
 /// Helper for determining the target from the host, etc. Tools may use this
 /// function to provide a consistent interpretation of the `--target=<string>`
 /// command-line option.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1d73397d330178..4e8fd44f526a5c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -348,6 +348,12 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
   if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
     opts.SaveTempsDir = a->getValue();
 
+  // -record-command-line option.
+  if (const llvm::opt::Arg *a =
+          args.getLastArg(clang::driver::options::OPT_record_command_line)) {
+    opts.RecordCommandLine = a->getValue();
+  }
+
   // -mlink-builtin-bitcode
   for (auto *a :
        args.filtered(clang::driver::options::OPT_mlink_builtin_bitcode))
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 5c86bd947ce73f..909e5909938d87 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -298,7 +298,7 @@ bool CodeGenAction::beginSourceFileAction() {
       kindMap, ci.getInvocation().getLoweringOpts(),
       ci.getInvocation().getFrontendOpts().envDefaults,
       ci.getInvocation().getFrontendOpts().features, targetMachine,
-      ci.getInvocation().getTargetOpts().cpuToTuneFor);
+      ci.getInvocation().getTargetOpts(), ci.getInvocation().getCodeGenOpts());
 
   // Fetch module from lb, so we can set
   mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ccbb481f472d81..2b42c3e355fbcc 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -6065,7 +6065,9 @@ Fortran::lower::LoweringBridge::LoweringBridge(
     const Fortran::lower::LoweringOptions &loweringOptions,
     const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
     const Fortran::common::LanguageFeatureControl &languageFeatures,
-    const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU)
+    const llvm::TargetMachine &targetMachine,
+    const Fortran::frontend::TargetOptions &targetOpts,
+    const Fortran::frontend::CodeGenOptions &cgOpts)
     : semanticsContext{semanticsContext}, defaultKinds{defaultKinds},
       intrinsics{intrinsics}, targetCharacteristics{targetCharacteristics},
       cooked{&cooked}, context{context}, kindMap{kindMap},
@@ -6122,11 +6124,13 @@ Fortran::lower::LoweringBridge::LoweringBridge(
   fir::setTargetTriple(*module.get(), triple);
   fir::setKindMapping(*module.get(), kindMap);
   fir::setTargetCPU(*module.get(), targetMachine.getTargetCPU());
-  fir::setTuneCPU(*module.get(), tuneCPU);
+  fir::setTuneCPU(*module.get(), targetOpts.cpuToTuneFor);
   fir::setTargetFeatures(*module.get(), targetMachine.getTargetFeatureString());
   fir::support::setMLIRDataLayout(*module.get(),
                                   targetMachine.createDataLayout());
   fir::setIdent(*module.get(), Fortran::common::getFlangFullVersion());
+  if (cgOpts.RecordCommandLine)
+    fir::setCommandline(*module.get(), *cgOpts.RecordCommandLine);
 }
 
 void Fortran::lower::genCleanUpInRegionIfAny(
diff --git a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
index 5bd8e2d4336361..01c0be66d1ecc3 100644
--- a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
+++ b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
@@ -130,6 +130,22 @@ llvm::StringRef fir::getIdent(mlir::ModuleOp mod) {
   return {};
 }
 
+void fir::setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine) {
+  if (cmdLine.empty())
+    return;
+
+  mlir::MLIRContext *ctx = mod.getContext();
+  mod->setAttr(mlir::LLVM::LLVMDialect::getCommandlineAttrName(),
+               mlir::StringAttr::get(ctx, cmdLine));
+}
+
+llvm::StringRef fir::getCommandline(mlir::ModuleOp mod) {
+  if (auto attr = mod->getAttrOfType<mlir::StringAttr>(
+          mlir::LLVM::LLVMDialect::getCommandlineAttrName()))
+    return attr;
+  return {};
+}
+
 std::string fir::determineTargetTriple(llvm::StringRef triple) {
   // Treat "" or "default" as stand-ins for the default machine.
   if (triple.empty() || triple == "default")
diff --git a/flang/test/Driver/frecord-command-line.f90 b/flang/test/Driver/frecord-command-line.f90
new file mode 100644
index 00000000000000..3dc65587ee2563
--- /dev/null
+++ b/flang/test/Driver/frecord-command-line.f90
@@ -0,0 +1,11 @@
+! This only checks that the command line is correctly passed on to the
+! -record-command-line option FC1 option and that the latter does not complain
+! about anything. The correct lowering to a module attribute and beyond will
+! be checked in other tests.
+!
+! RUN: %flang -### -frecord-command-line %s 2>&1 | FileCheck --check-prefix=CHECK-RECORD ...
[truncated]

Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MLIR part looks good to me, but I cannot speak for clang nor flang 🙂

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor questions.

clang/include/clang/Driver/Options.td Show resolved Hide resolved
@@ -1980,9 +1980,11 @@ within a command line are combined with spaces; spaces and backslashes within an
argument are escaped with backslashes. This format differs from the format of
the equivalent section produced by GCC with the -frecord-gcc-switches flag.
This option is currently only supported on ELF targets.}]>,
Group<f_clang_Group>;
Group<f_clang_Group>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know whether flang's flow is affected by f_clang_Group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify what you mean by "flang's flow"?

I don't know exactly what f_clang_Group does. I haven't been able to determine what effect it has on things like help messages. For instance -frecord-command-line does not appear in either clang --help, or clang --help-hidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kiranchandramohan, gentle ping

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean't whether f_clang_Group is processed by flang or not. I see that this is used by some other commands as well. So it is alrite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think f_clang_Group is a legacy group for Clang options not in GCC. f_clang_Group has never gain significant adoption and it's used by any code.

New -f options can just use f_Group.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed the group to f_Group since that does not suggest that the options are exclusive to clang.

@tarunprabhu tarunprabhu force-pushed the add-llvm-command-metadata branch 2 times, most recently from 79976c8 to 6df2f16 Compare September 5, 2024 20:50
Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@MaskRay
Copy link
Member

MaskRay commented Sep 16, 2024

Why do we need the two new files?

(added) clang/lib/Driver/ToolChains/CommonUtils.cpp (+76)
(added) clang/lib/Driver/ToolChains/CommonUtils.h (+44)

We have clang/lib/Driver/ToolChains/CommonArgs.cpp and clang/lib/Driver/ToolChain.cpp for shared code.

@tarunprabhu
Copy link
Contributor Author

Why do we need the two new files?

We have clang/lib/Driver/ToolChains/CommonArgs.cpp and clang/lib/Driver/ToolChain.cpp for shared code.

Since most of the code in CommonArgs.cpp had to do with functions in the clang::driver::tools namespace, I wasn't sure that code that is only shared between Clang.cpp and Flang.cpp ought to go there. The other function that was moved is a utility function to escape arguments and was originally declared static in Clang.cpp.

If it is ok to use CommandArgs.cpp for limited sharing and utilities, I can move the code there.

@tarunprabhu tarunprabhu force-pushed the add-llvm-command-metadata branch from a77d788 to 367519d Compare September 16, 2024 19:28
@MaskRay
Copy link
Member

MaskRay commented Sep 17, 2024

Why do we need the two new files?
We have clang/lib/Driver/ToolChains/CommonArgs.cpp and clang/lib/Driver/ToolChain.cpp for shared code.

Since most of the code in CommonArgs.cpp had to do with functions in the clang::driver::tools namespace, I wasn't sure that code that is only shared between Clang.cpp and Flang.cpp ought to go there. The other function that was moved is a utility function to escape arguments and was originally declared static in Clang.cpp.

If it is ok to use CommandArgs.cpp for limited sharing and utilities, I can move the code there.

Adding the shared code to CommonArgs.cpp LGTM. Shared functions there use tools::, though.

rg driver:: clang/lib/Driver reveals what files define driver:: member functions. CommonArgs.cpp doesn't define driver::.

@tarunprabhu
Copy link
Contributor Author

Moved code to the clang::driver::tools namespace

@tarunprabhu
Copy link
Contributor Author

@MaskRay, If everything looks good to you now, could you approve this?

I apologize for being pushy, but this will help us get moving on some other things in flang. Thanks.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

As you are renaming functions, you can fix the function naming style
tools::function instead of tools::Function.
While the style is largely inconsistent, we usually fix the style while renaming functions.

@tarunprabhu
Copy link
Contributor Author

As you are renaming functions, you can fix the function naming style tools::function instead of tools::Function. While the style is largely inconsistent, we usually fix the style while renaming functions.

I will do that and merge. Thanks!

Tarun Prabhu added 6 commits September 19, 2024 12:02
Add support for the -frecord-command-line option that will produce the
llvm.commandline metadata which will eventually be saved in the object file.
This behavior is also supported in clang. Some refactoring of the code in flang
to handle these command line options was carried out. The corresponding
-grecord-command-line option which saves the command line in the debug
information has not yet been enabled for flang.
…the code

to CommonArgs instead. That code has been put in the clang::driver namespace.
@tarunprabhu tarunprabhu force-pushed the add-llvm-command-metadata branch from 17161f4 to 4e2ceec Compare September 19, 2024 21:35
@tarunprabhu tarunprabhu merged commit b3533a1 into llvm:main Sep 20, 2024
8 checks passed
@tarunprabhu tarunprabhu deleted the add-llvm-command-metadata branch September 20, 2024 00:28
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 20, 2024

LLVM Buildbot has detected a new failure on builder flang-aarch64-libcxx running on linaro-flang-aarch64-libcxx while building clang,flang,mlir at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/6766

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Driver/bbc-mlir-pass-pipeline.f90' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 3: bbc --mlir-pass-statistics --mlir-pass-statistics-display=pipeline /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90 2>&1 | /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90
+ bbc --mlir-pass-statistics --mlir-pass-statistics-display=pipeline /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90
+ /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90
/home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90:9:10: error: CHECK: expected string not found in input
! CHECK: Pass statistics report
         ^
<stdin>:1:1: note: scanning from here
: CommandLine Error: Option 'fdynamic-heap-array' registered more than once!
^
<stdin>:2:14: note: possible intended match here
LLVM ERROR: inconsistency in registered CommandLine options
             ^

Input file: <stdin>
Check file: /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           1: : CommandLine Error: Option 'fdynamic-heap-array' registered more than once! 
check:9'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
           2: LLVM ERROR: inconsistency in registered CommandLine options 
check:9'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:9'1                  ?                                               possible intended match
>>>>>>

--

********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 20, 2024

LLVM Buildbot has detected a new failure on builder flang-aarch64-latest-gcc running on linaro-flang-aarch64-latest-gcc while building clang,flang,mlir at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/130/builds/3812

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Driver/bbc-mlir-pass-pipeline.f90' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 3: bbc --mlir-pass-statistics --mlir-pass-statistics-display=pipeline /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90 2>&1 | /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90
+ bbc --mlir-pass-statistics --mlir-pass-statistics-display=pipeline /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90
+ /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90
/home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90:9:10: error: CHECK: expected string not found in input
! CHECK: Pass statistics report
         ^
<stdin>:1:1: note: scanning from here
: CommandLine Error: Option 'fdynamic-heap-array' registered more than once!
^
<stdin>:2:14: note: possible intended match here
LLVM ERROR: inconsistency in registered CommandLine options
             ^

Input file: <stdin>
Check file: /home/tcwg-buildbot/worker/flang-aarch64-latest-gcc/llvm-project/flang/test/Driver/bbc-mlir-pass-pipeline.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           1: : CommandLine Error: Option 'fdynamic-heap-array' registered more than once! 
check:9'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
           2: LLVM ERROR: inconsistency in registered CommandLine options 
check:9'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:9'1                  ?                                               possible intended match
>>>>>>

--

********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 20, 2024

LLVM Buildbot has detected a new failure on builder flang-aarch64-sharedlibs running on linaro-flang-aarch64-sharedlibs while building clang,flang,mlir at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/80/builds/3854

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Driver/bbc-cuda-macro.cuf' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
RUN: at line 2: bbc -fcuda -o - /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/llvm-project/flang/test/Driver/bbc-cuda-macro.cuf | /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/llvm-project/flang/test/Driver/bbc-cuda-macro.cuf
+ bbc -fcuda -o - /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/llvm-project/flang/test/Driver/bbc-cuda-macro.cuf
+ /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/llvm-project/flang/test/Driver/bbc-cuda-macro.cuf
: CommandLine Error: Option 'fdynamic-heap-array' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-sharedlibs/llvm-project/flang/test/Driver/bbc-cuda-macro.cuf

--

********************


DavidSpickett added a commit that referenced this pull request Sep 20, 2024
@DavidSpickett
Copy link
Collaborator

I've had to revert this due to the failures listed above. To reproduce, configure with -DBUILD_SHARED_LIBS=ON.

(the other 2 bots are not called shared libs but they do enable this setting too)

$ ./bin/bbc
: CommandLine Error: Option 'fdynamic-heap-array' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options
Aborted (core dumped)

This happens to at least the bbc and tco tools that I notice include a .inc file related to commands. So that might be the issue, but simply removing the include wasn't correct so I leave it to you to figure out.

@tarunprabhu
Copy link
Contributor Author

Thanks for the instructions on how to reproduce. I'll take a look.

@tarunprabhu
Copy link
Contributor Author

The underlying issue is due to CLOptions.inc being included in FrontendActions.cpp and bbc.cpp. The file contained a number of command line options and functions, all of which were declared static. This PR added libflangFrontend to the list of libraries linked into bbc which resulted in duplicate definition errors. The underlying issue can be solved by avoiding including files containing definitions directly in another. #109874 does this. It should be safe to reland this after that is merged.

xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
tarunprabhu added a commit that referenced this pull request Oct 14, 2024
…102975)"

The underlying issue was caused by a file included in two different
places which resulted in duplicate definition errors when linking
individual shared libraries. This was fixed in c3201dd
[#109874].
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…lvm#102975)"

The underlying issue was caused by a file included in two different
places which resulted in duplicate definition errors when linking
individual shared libraries. This was fixed in c3201dd
[llvm#109874].
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
…lvm#102975)"

The underlying issue was caused by a file included in two different
places which resulted in duplicate definition errors when linking
individual shared libraries. This was fixed in c3201dd
[llvm#109874].
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
…lvm#102975)"

The underlying issue was caused by a file included in two different
places which resulted in duplicate definition errors when linking
individual shared libraries. This was fixed in c3201dd
[llvm#109874].
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang:fir-hlfir flang Flang issues not falling into any other category mlir:llvm mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants