[flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line

[mjklemm]

This PR adds the -fno-fortran-main command line option to remove Fortran_main.a from the link and to allow for linking Fortran code w/o program unit with C/C++ translation units that provide the main() entrypoint.

When linking Fortran code with C/C++ code (Fortran calling into C/C++), PR #73124 introduced a proper error message that would prevent successful linkage, if there was a program unit from Fortran and main() function coming from C/C++. Alas, this caused some breakage of code that would call Fortran code from C/C++ and rightfully provided the main() entrypoint. Classic Flang had the command-line option -fno-fortran-main to then remove the entrypoints for the Fortran program unit from the linker stage.

This PR is related to PR #74120 and (merged) PR #73124.

[llvmbot]

@llvm/pr-subscribers-flang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Michael Klemm (mjklemm)

Changes

This is related to PR #74120 and (merged) PR #73124.

This PR adds the -fno-fortran-main command line option to remove Fortran_main.a from the link and to allow for linking Fortran code w/o program unit with C/C++ translation units that provide the main() entrypoint.

---

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

2 Files Affected:

• (modified) clang/include/clang/Driver/Options.td (+4)
• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+25-21)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 19d04e82aed4d68..aa26344f67b3132 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">,
   Group<gfortran_Group>,
   Alias<module_dir>;
 
+def no_fortran_main : Flag<["-"], "fno-fortran-main">,
+  Visibility<[FlangOption]>, Group<f_Group>,
+  HelpText<"Don't link in Fortran main">;
+
 //===----------------------------------------------------------------------===//
 // FC1 Options
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 0ae8e2dce32e94a..2899f07cb7490ca 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
   return true;
 }
 
+// TODO: add -fno-fortran-main option and check it in this function.
 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 (!TC.getTriple().isKnownWindowsMSVCEnvironment()) {
-    // 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;
-        }
-
-    if (!WholeArchiveActive)
-      CmdArgs.push_back("--whole-archive");
-    CmdArgs.push_back("-lFortran_main");
-    if (!WholeArchiveActive)
-      CmdArgs.push_back("--no-whole-archive");
+    // if -fno-fortran-main has been passed, skip linking Fortran_main.a
+    bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr;
+    if (!DontLinkFortranMain) {
+      // 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;
+          }
 
+      if (!WholeArchiveActive)
+        CmdArgs.push_back("--whole-archive");
+      CmdArgs.push_back("-lFortran_main");
+      if (!WholeArchiveActive)
+        CmdArgs.push_back("--no-whole-archive");
+    }
     // Perform regular linkage of the remaining runtime libraries.
     CmdArgs.push_back("-lFortranRuntime");
     CmdArgs.push_back("-lFortranDecimal");

[github-actions]

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

[mjklemm]

This WIP for now, as I still have to make the changes for MSVC. But it's good enough to discuss if this is a viable solution.

[DavidTruby]

I think this solution is fine, at least in the short term.

I had a think after reviewing the initial patch and looking at the failure that @tblah showed in #73124; my thoughts are that the “correct” way of doing this would be instead of linking Fortran_main all the time, we could insert a linker directive in the object file containing the program statement. That way we would only link Fortran_main when there is actually a program statement in the Fortran. However that’s more work and would need a bit more thought anyway so we at least need something like this or the revert in the interim.

[mjklemm]

I think this solution is fine, at least in the short term.

I had a think after reviewing the initial patch and looking at the failure that @tblah showed in #73124; my thoughts are that the “correct” way of doing this would be instead of linking Fortran_main all the time, we could insert a linker directive in the object file containing the program statement. That way we would only link Fortran_main when there is actually a program statement in the Fortran. However that’s more work and would need a bit more thought anyway so we at least need something like this or the revert in the interim.

That sounds good! For now, the code should effectively avoid linking Fortran_main on Linux and Windows if -fno-fortran-main is present.

[tblah]

Thanks for the fix. LGTM

[mjklemm]

@DavidTruby If you are OK with the way I handled MSVC, please approve and I will merge the PR (or change it if you want some changes to be made).

[banach-space]

Thanks for pushing on!

I've made one small suggestion and I also have one more request for the summary. I know that you added links to relevant PRs, but I would still appreciate if the summary (and the commit msg) where self-contained and included the justification for this new flag. You could use the example by @tblah to show where the flag might be needed. Please do keep the links :)

I think that it would also be helpful to include what could be an alternative to this and that wouldn't require a flag - I think that the only option would be to mimic what Classic Flang does?

I know that it's extra work, but it's also super useful bit of documentation. And I would only rarely try to track the history beyond commit messages (there's just too much otherwise).

[mjklemm]

I know that it's extra work, but it's also super useful bit of documentation. And I would only rarely try to track the history beyond commit messages (there's just too much otherwise).

I can absolutely do that. I'll craft some wording for the rationale behind that change and add it to the PR.

[DavidTruby]

LGTM sorry sometimes reviews get lost in the noise on GitHub...

[banach-space]

I'm making two more small suggestions, but these are nits. Fell free to ignore.

LGTM, thank you!

[nit] "legacy flang" --> "Classic Flang" (in summary)