[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
banach-space wrote: > 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. https://github.com/llvm/llvm-project/pull/75534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
https://github.com/psteinfeld commented: 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) ``` https://github.com/llvm/llvm-project/pull/75534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
@@ -1116,73 +1116,87 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
+/// Determines if --whole-archive is active in the list of arguments.
+static bool isWholeArchivePresent(const ArgList ) {
+ bool WholeArchiveActive = false;
+ for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) {
+if (Arg) {
mjklemm wrote:
Thanks!
https://github.com/llvm/llvm-project/pull/75534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
https://github.com/banach-space closed https://github.com/llvm/llvm-project/pull/75534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
@@ -1116,73 +1116,87 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
+/// Determines if --whole-archive is active in the list of arguments.
+static bool isWholeArchivePresent(const ArgList ) {
+ bool WholeArchiveActive = false;
+ for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) {
+if (Arg) {
banach-space wrote:
I find this to be a borderline case, so just opted for whatever feels more
readable :)
Some relevant bits from the [coding
standard](https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements)
> However, braces should be used in cases where the omission of braces harm the
> readability and maintainability of the code.
```cpp
// Use braces on the outer block because there are more than two levels of
// nesting.
if (isa(D)) {
for (auto *A : D.attrs())
for (ssize_t i : llvm::seq(count))
handleAttrOnDecl(D, A, i);
}
```
```cpp
// Use braces on the outer block because of a nested `if`; otherwise the
// compiler would warn: `add explicit braces to avoid dangling else`
if (auto *D = dyn_cast(D)) {
if (shouldProcess(D))
handleVarDecl(D);
else
markAsIgnored(D);
}
```
https://github.com/llvm/llvm-project/pull/75534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
@@ -1116,73 +1116,87 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
+/// Determines if --whole-archive is active in the list of arguments.
+static bool isWholeArchivePresent(const ArgList ) {
+ bool WholeArchiveActive = false;
+ for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) {
+if (Arg) {
mjklemm wrote:
Nit (and maybe more a question): The style guide does not use {} when only one
statement is nested. Is that true? (Frankly, I like the extra {} better, as it
makes adding code easier).
https://github.com/llvm/llvm-project/pull/75534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
https://github.com/mjklemm approved this pull request. LGTM! Thanks for the refactoring. This makes the code much easier to digest! https://github.com/llvm/llvm-project/pull/75534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][nfc] Refactor linker invocation logic (PR #75534)
https://github.com/banach-space updated
https://github.com/llvm/llvm-project/pull/75534
From b0b3bc42f0d6cb881ee0d028d2cc358ba3a347ed Mon Sep 17 00:00:00 2001
From: Andrzej Warzynski
Date: Thu, 14 Dec 2023 21:34:11 +
Subject: [PATCH] [flang][nfc] Refactor linker invocation logic
Refactor how the Fortran runtime libs are added to the linker
invocation. This is a non-functional change.
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 136 -
flang/test/Driver/linker-flags.f90 | 8 +-
2 files changed, 79 insertions(+), 65 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 3d1df58190ce05..6de41642a734a7 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1116,73 +1116,87 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
+/// Determines if --whole-archive is active in the list of arguments.
+static bool isWholeArchivePresent(const ArgList ) {
+ 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 ,
+ llvm::opt::ArgStringList ) {
+ unsigned RTOptionID = options::OPT__SLASH_MT;
+ if (auto *rtl = Args.getLastArg(options::OPT_fms_runtime_lib_EQ)) {
+RTOptionID = llvm::StringSwitch(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 , const ArgList ,
llvm::opt::ArgStringList ) {
- // 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) {
-
