[flang][nfc] Refactor linker invocation logic

[banach-space]

Refactor how the Fortran runtime libs are added to the linker
invocation. This is a non-functional change.

This is an updated version of #75534. This iteration makes sure that
FortranMain.a comes before FortranRuntme.a (the former depends on the
latter).

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Andrzej Warzyński (banach-space)

Changes

Refactor how the Fortran runtime libs are added to the linker
invocation. This is a non-functional change.

This is an updated version of #75534. This iteration makes sure that
FortranMain.a comes before FortranRuntme.a (the former depends on the
latter).

---

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

1 Files Affected:

• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+82-63)

diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 3d1df58190ce05..45901ee7157f77 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1116,72 +1116,91 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
   return true;
 }
 
-void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
-                                  llvm::opt::ArgStringList &CmdArgs) {
-  // These are handled earlier on Windows by telling the frontend driver to add
-  // the correct libraries to link against as dependents in the object file.
-
-  // if -fno-fortran-main has been passed, skip linking Fortran_main.a
-  bool LinkFortranMain = !Args.hasArg(options::OPT_no_fortran_main);
-  if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) {
-    if (LinkFortranMain) {
-      // The --whole-archive option needs to be part of the link line to
-      // make sure that the main() function from Fortran_main.a is pulled
-      // in by the linker.  Determine if --whole-archive is active when
-      // flang will try to link Fortran_main.a.  If it is, don't add the
-      // --whole-archive flag to the link line.  If it's not, add a proper
-      // --whole-archive/--no-whole-archive bracket to the link line.
-      bool WholeArchiveActive = false;
-      for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) {
-        if (Arg) {
-          for (StringRef ArgValue : Arg->getValues()) {
-            if (ArgValue == "--whole-archive")
-              WholeArchiveActive = true;
-            if (ArgValue == "--no-whole-archive")
-              WholeArchiveActive = false;
-          }
-        }
+/// Determines if --whole-archive is active in the list of arguments.
+static bool isWholeArchivePresent(const ArgList &Args) {
+  bool WholeArchiveActive = false;
+  for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) {
+    if (Arg) {
+      for (StringRef ArgValue : Arg->getValues()) {
+        if (ArgValue == "--whole-archive")
+          WholeArchiveActive = true;
+        if (ArgValue == "--no-whole-archive")
+          WholeArchiveActive = false;
       }
+    }
+  }
 
-      // TODO: Find an equivalent of `--whole-archive` for Darwin.
-      if (!WholeArchiveActive && !TC.getTriple().isMacOSX()) {
-        CmdArgs.push_back("--whole-archive");
-        CmdArgs.push_back("-lFortran_main");
-        CmdArgs.push_back("--no-whole-archive");
-      } else {
-        CmdArgs.push_back("-lFortran_main");
-      }
+  return WholeArchiveActive;
+}
 
-      // Perform regular linkage of the remaining runtime libraries.
-      CmdArgs.push_back("-lFortranRuntime");
-      CmdArgs.push_back("-lFortranDecimal");
-    }
-  } else {
-    if (LinkFortranMain) {
-      unsigned RTOptionID = options::OPT__SLASH_MT;
-      if (auto *rtl = Args.getLastArg(options::OPT_fms_runtime_lib_EQ)) {
-        RTOptionID = llvm::StringSwitch<unsigned>(rtl->getValue())
-                         .Case("static", options::OPT__SLASH_MT)
-                         .Case("static_dbg", options::OPT__SLASH_MTd)
-                         .Case("dll", options::OPT__SLASH_MD)
-                         .Case("dll_dbg", options::OPT__SLASH_MDd)
-                         .Default(options::OPT__SLASH_MT);
-      }
-      switch (RTOptionID) {
-      case options::OPT__SLASH_MT:
-        CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.static.lib");
-        break;
-      case options::OPT__SLASH_MTd:
-        CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.static_dbg.lib");
-        break;
-      case options::OPT__SLASH_MD:
-        CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.dynamic.lib");
-        break;
-      case options::OPT__SLASH_MDd:
-        CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.dynamic_dbg.lib");
-        break;
-      }
-    }
+/// Add Fortran runtime libs for MSVC
+static void addFortranRuntimeLibsMSVC(const ArgList &Args,
+                                      llvm::opt::ArgStringList &CmdArgs) {
+  unsigned RTOptionID = options::OPT__SLASH_MT;
+  if (auto *rtl = Args.getLastArg(options::OPT_fms_runtime_lib_EQ)) {
+    RTOptionID = llvm::StringSwitch<unsigned>(rtl->getValue())
+                     .Case("static", options::OPT__SLASH_MT)
+                     .Case("static_dbg", options::OPT__SLASH_MTd)
+                     .Case("dll", options::OPT__SLASH_MD)
+                     .Case("dll_dbg", options::OPT__SLASH_MDd)
+                     .Default(options::OPT__SLASH_MT);
+  }
+  switch (RTOptionID) {
+  case options::OPT__SLASH_MT:
+    CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.static.lib");
+    break;
+  case options::OPT__SLASH_MTd:
+    CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.static_dbg.lib");
+    break;
+  case options::OPT__SLASH_MD:
+    CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.dynamic.lib");
+    break;
+  case options::OPT__SLASH_MDd:
+    CmdArgs.push_back("/WHOLEARCHIVE:Fortran_main.dynamic_dbg.lib");
+    break;
+  }
+}
+
+// Add FortranMain runtime lib
+static void addFortranMain(const ToolChain &TC, const ArgList &Args,
+                           llvm::opt::ArgStringList &CmdArgs) {
+  // 1. MSVC
+  if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
+    addFortranRuntimeLibsMSVC(Args, CmdArgs);
+    return;
+  }
+
+  // 2. GNU and similar
+  // The --whole-archive option needs to be part of the link line to make
+  // sure that the main() function from Fortran_main.a is pulled in by the
+  // linker. However, it shouldn't be used if it's already active.
+  // TODO: Find an equivalent of `--whole-archive` for Darwin.
+  if (!isWholeArchivePresent(Args) && !TC.getTriple().isMacOSX()) {
+    CmdArgs.push_back("--whole-archive");
+    CmdArgs.push_back("-lFortran_main");
+    CmdArgs.push_back("--no-whole-archive");
+    return;
+  }
+
+  CmdArgs.push_back("-lFortran_main");
+}
+
+/// Add Fortran runtime libs
+void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
+                                  llvm::opt::ArgStringList &CmdArgs) {
+  // 1. Link FortranMain
+  // FortranMain depends on FortranRuntime, so needs to be listed first. If
+  // -fno-fortran-main has been passed, skip linking Fortran_main.a
+  if (!Args.hasArg(options::OPT_no_fortran_main))
+    addFortranMain(TC, Args, CmdArgs);
+
+  // 2. Link FortranRuntime and FortranDecimal
+  // These are handled earlier on Windows by telling the frontend driver to
+  // add the correct libraries to link against as dependents in the object
+  // file.
+  if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) {
+    CmdArgs.push_back("-lFortranRuntime");
+    CmdArgs.push_back("-lFortranDecimal");
   }
 }
 

[mjklemm]

Looks good to me. Did touch testing with my reproducers and it worked as I would expect it.

[psteinfeld]

All looks good.

Thanks for the quick fix!