[flang][nfc] Refactor linker invocation logic #75534
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-flang-driver Author: Andrzej Warzyński (banach-space) ChangesRefactor how the Fortran runtime libs are added to the linker Full diff: https://github.com/llvm/llvm-project/pull/75534.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 3d1df58190ce05..35f3db31db9b8f 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1116,73 +1116,87 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
return true;
}
+/// 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;
+ }
+ }
+ }
+
+ return WholeArchiveActive;
+}
+
+/// 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 Fortran runtime libs
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);
+ // 1. 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()) {
- 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;
- }
- }
- }
+ CmdArgs.push_back("-lFortranRuntime");
+ CmdArgs.push_back("-lFortranDecimal");
+ }
- // 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");
- }
+ // 2. Link FortranMain
+ // If -fno-fortran-main has been passed, skip linking Fortran_main.a
+ if (Args.hasArg(options::OPT_no_fortran_main))
+ return;
- // 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;
- }
- }
+ // 2.1. MSVC
+ if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
+ addFortranRuntimeLibsMSVC(Args, CmdArgs);
+ return;
}
+
+ // 2.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");
}
void tools::addFortranRuntimeLibraryPath(const ToolChain &TC,
diff --git a/flang/test/Driver/linker-flags.f90 b/flang/test/Driver/linker-flags.f90
index ea91946316cfaa..31e7ab71aacdb1 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -28,23 +28,23 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "--whole-archive" "-lFortran_main" "--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal" "-lm"
+! UNIX-SAME: "-lFortranRuntime" "-lFortranDecimal" "--whole-archive" "-lFortran_main" "--no-whole-archive" "-lm"
! DARWIN-LABEL: "{{.*}}ld{{(\.exe)?}}"
! DARWIN-SAME: "[[object_file]]"
-! DARWIN-SAME: -lFortran_main
! DARWIN-SAME: -lFortranRuntime
! DARWIN-SAME: -lFortranDecimal
+! DARWIN-SAME: -lFortran_main
! HAIKU-LABEL: "{{.*}}ld{{(\.exe)?}}"
! HAIKU-SAME: "[[object_file]]"
-! HAIKU-SAME: "--whole-archive" "-lFortran_main" "--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal"
+! HAIKU-SAME: "-lFortranRuntime" "-lFortranDecimal" "--whole-archive" "-lFortran_main" "--no-whole-archive"
! MINGW-LABEL: "{{.*}}ld{{(\.exe)?}}"
! MINGW-SAME: "[[object_file]]"
-! MINGW-SAME: -lFortran_main
! MINGW-SAME: -lFortranRuntime
! MINGW-SAME: -lFortranDecimal
+! MINGW-SAME: -lFortran_main
! NOTE: This also matches lld-link (when CLANG_DEFAULT_LINKER=lld) and
! any .exe suffix that is added when resolving to the full path of
|
|
@llvm/pr-subscribers-clang Author: Andrzej Warzyński (banach-space) ChangesRefactor how the Fortran runtime libs are added to the linker Full diff: https://github.com/llvm/llvm-project/pull/75534.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 3d1df58190ce05..35f3db31db9b8f 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1116,73 +1116,87 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
return true;
}
+/// 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;
+ }
+ }
+ }
+
+ return WholeArchiveActive;
+}
+
+/// 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 Fortran runtime libs
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);
+ // 1. 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()) {
- 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;
- }
- }
- }
+ CmdArgs.push_back("-lFortranRuntime");
+ CmdArgs.push_back("-lFortranDecimal");
+ }
- // 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");
- }
+ // 2. Link FortranMain
+ // If -fno-fortran-main has been passed, skip linking Fortran_main.a
+ if (Args.hasArg(options::OPT_no_fortran_main))
+ return;
- // 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;
- }
- }
+ // 2.1. MSVC
+ if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
+ addFortranRuntimeLibsMSVC(Args, CmdArgs);
+ return;
}
+
+ // 2.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");
}
void tools::addFortranRuntimeLibraryPath(const ToolChain &TC,
diff --git a/flang/test/Driver/linker-flags.f90 b/flang/test/Driver/linker-flags.f90
index ea91946316cfaa..31e7ab71aacdb1 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -28,23 +28,23 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "--whole-archive" "-lFortran_main" "--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal" "-lm"
+! UNIX-SAME: "-lFortranRuntime" "-lFortranDecimal" "--whole-archive" "-lFortran_main" "--no-whole-archive" "-lm"
! DARWIN-LABEL: "{{.*}}ld{{(\.exe)?}}"
! DARWIN-SAME: "[[object_file]]"
-! DARWIN-SAME: -lFortran_main
! DARWIN-SAME: -lFortranRuntime
! DARWIN-SAME: -lFortranDecimal
+! DARWIN-SAME: -lFortran_main
! HAIKU-LABEL: "{{.*}}ld{{(\.exe)?}}"
! HAIKU-SAME: "[[object_file]]"
-! HAIKU-SAME: "--whole-archive" "-lFortran_main" "--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal"
+! HAIKU-SAME: "-lFortranRuntime" "-lFortranDecimal" "--whole-archive" "-lFortran_main" "--no-whole-archive"
! MINGW-LABEL: "{{.*}}ld{{(\.exe)?}}"
! MINGW-SAME: "[[object_file]]"
-! MINGW-SAME: -lFortran_main
! MINGW-SAME: -lFortranRuntime
! MINGW-SAME: -lFortranDecimal
+! MINGW-SAME: -lFortran_main
! NOTE: This also matches lld-link (when CLANG_DEFAULT_LINKER=lld) and
! any .exe suffix that is added when resolving to the full path of
|
|
Refactor how the Fortran runtime libs are added to the linker invocation. This is a non-functional change.
banach-space
force-pushed
the
andrzej/runtime_libs
branch
from
966e22e
to
b0b3bc4
Compare
mjklemm
approved these changes
Dec 15, 2023
preames
added a commit
that referenced
this pull request
Dec 15, 2023
This reverts commit 71bbfab. Breaks check-flang on x86_64 host.
psteinfeld
reviewed
Dec 15, 2023
There was a problem hiding this comment.
After this change, I can no longer use flang-new to create an executable program. When I try, I get the following output:
[psteinfeld@ice4 build]$ flang-new hello.f90
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../bin/ld: /local/home/psteinfeld/up/build/lib/libFortran_main.a(Fortran_main.c.o): in function `main':
Fortran_main.c:(.text.startup.main+0xf): undefined reference to `_FortranAProgramStart'
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../bin/ld: Fortran_main.c:(.text.startup.main+0x19): undefined reference to `_FortranAProgramEndStatement'
flang-new: error: linker command failed with exit code 1 (use -v to see invocation)
Sorry about that Pete :( This has already been reverted, so things should work just fine if you pull and rebuild. Not sure why pre-commit CI didn't catch that. |
banach-space
added a commit
to banach-space/llvm-project
that referenced
this pull request
Dec 15, 2023
Refactor how the Fortran runtime libs are added to the linker invocation. This is a non-functional change. This is an updated version of llvm#75534. This iteration makes sure that FortranMain.a comes before FortranRuntme.a (the former depends on the latter).
|
Updated version: #75648 |
banach-space
added a commit
that referenced
this pull request
Dec 16, 2023
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).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor how the Fortran runtime libs are added to the linker
invocation. This is a non-functional change.