[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
banach-space wrote: @kkwli and @DanielCChen - could you review https://github.com/llvm/llvm-project/pull/75816 and see whether that makes sense to you? I am referring specifically to the documentation that @mjklemm is kindly adding. Hopefully that will help folks avoid similar issues in the future. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
kkwli wrote:
Yep the situation is something like using `flang-new` to link the C and Fortran
objects.
```
$ cat main.c
void fsub();
int main() { fsub(); }
$ cat sub.f90
subroutine fsub()
end subroutine
$ flang-new -c sub.f90 -fno-underscoring
$ clang -c main.c
$ flang-new main.o sub.o
ld.lld: error: duplicate symbol: main
>>> defined at main.c
>>>main.o:(main)
>>> defined at Fortran_main.c:18
>>> (llvm-project/flang/runtime/FortranMain/Fortran_main.c:18)
>>>Fortran_main.c.o:(.text+0x0) in archive
>>> /scratch/kli/wrk/f/build-flang/lib/libFortran_main.a
flang-new: error: linker command failed with exit code 1 (use -v to see
invocation)
$ flang-new main.o sub.o -fno-fortran-main
$ echo $?
0
```
It is good that this situation is caught by most linkers (unfortunately, AIX
linker does not complain and picks up one of the `main`s!) Requiring
`-fno-fortran-main` for this situation is different from what gfortran behaves.
It may make porting code to flang less convenient. In my opinion, it is
probably helpful to document this subtle difference somewhere.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
DanielCChen wrote: The test cases actually have C main indeed and call to Fortran procedures as opposed to what I thought (the other way around). Adding `-fno-fortran-main` fixed all of them! May be I missed it when reading through the comments of this PR, why there is a definition of `main()` being generated on the Fortran side when the Fortran sources are procedures or modules? https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
banach-space wrote: > I see. So Fortran and C interoperability of F2003/F2008 is not supported yet > in Flang? That's not really what I had in mind. It worked for you so everything that's needed is there, but no upstream testing indicates that you might be the first person trying it. And that nobody has actually tried verifying it and making sure that relevant various bits are integrated in the intended way. > Those ~100ish regression test cases we have were passing before this PR > though. That's a lot - I am sorry about that :( > I think I can copy the source code of one test case here, but it needs to be > run manually. It sounds like you are hitting something fairly fundamental that should be reproducible with a couple of files (C and Fortran), few lines each. And without a repro, it will be very hard to fix that. We could revert this change, but then again - without a simple repro we won't be able to improve this for the future. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
DanielCChen wrote: I see. So Fortran and C interoperability of F2003/F2008 is not supported yet in Flang? Those ~100ish regression test cases we have were passing before this PR though. Unfortunately, those test cases are not made public available yet. I think I can copy the source code of one test case here, but it needs to be run manually. Please let me know if that is desired to help debug the reason of the regression. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
DanielCChen wrote: @mjklemm This PR caused some regressions of C-interop test cases in our local test run. The test cases typically have a Fortran main (compiled with Flang) that calls a C function (compiled with clang). The linking is by `flang-new`. The error looks like: ``` ld.lld: error: duplicate symbol: main >>> defined at ./bind_c09i.c >>>bind_c09i.o:(.The_Code) >>> defined at Fortran_main.c >>>Fortran_main.c.o:(.text.main+0x0) in archive ./libFortran_main.a flang-new: error: linker command failed with exit code 1 (use -v to see invocation) ``` https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > We are also seeing the same issue when linking on Mac regarding the ld: > unknown options: --whole-archive Is there an equivalent option for the MacOS linker? https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
banach-space wrote: > It seems like flang-new when being used as a linker with -shared included > Fortran_main in the shared library. This seems wrong to me. I am trying to recall the rationale behind that, but it's been a while :( Here's a relevant discussion/bug that hasn't been resolved yet. It might feel off-topic at first, but it's in fact a very similar problem - where and when should `main` be defined? It would be incredibly helpful if somebody resolved that 🙏🏻 . https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: Thanks for the reproducer. > The error shows up when linking a C program with a Fortran shared library, so > maybe you weren't enabling building shared libraries? I was building OpenBLAS using the "old" Makefile-based build. There the issue indeed does not happen. When I switched to your CMake configuration line, I was able to reproduce the problem. The solution is to add `-fno-fortran-main` to the linker options via `CMAKE_SHARED_LINKER_FLAGS`. This will need PR https://github.com/llvm/llvm-project/pull/74139 land first. But this option will be a good way to control if the flang compiler should attempt linking in the `main` stub from its library. It seems like `flang-new` when being used as a linker with `-shared` included Fortran_main in the shared library. This seems wrong to me. The option `-fno-fortran-main` avoids this. I'm pondering if `-shared` is buggy here. It will require a bit more digging on my end to figure that out. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
rj-jesus wrote: Chipping into the discussion, since this patch I can also no longer build OpenBLAS or PETSc. OpenBLAS for example fails with ``` $ clang -v -O3 -mcpu=native -DHAVE_C11 -Wall -DF_INTERFACE_GFORT -fPIC -DSMP_SERVER -DNO_WARMUP -DMAX_CPU_NUMBER=72 -DMAX_PARALLEL_NUMBER=1 -DMAX_STACK_ALLOC=2048 -DNO_AFFINITY -DVERSION="\"0.3.25\"" -DBUILD_SINGLE -DBUILD_DOUBLE -DBUILD_COMPLEX -DBUILD_COMPLEX16 utest/CMakeFiles/openblas_utest.dir/utest_main.c.o utest/CMakeFiles/openblas_utest.dir/test_min.c.o utest/CMakeFiles/openblas_utest.dir/test_amax.c.o utest/CMakeFiles/openblas_utest.dir/test_ismin.c.o utest/CMakeFiles/openblas_utest.dir/test_rotmg.c.o utest/CMakeFiles/openblas_utest.dir/test_rot.c.o utest/CMakeFiles/openblas_utest.dir/test_axpy.c.o utest/CMakeFiles/openblas_utest.dir/test_dsdot.c.o utest/CMakeFiles/openblas_utest.dir/test_dnrm2.c.o utest/CMakeFiles/openblas_utest.dir/test_swap.c.o utest/CMakeFiles/openblas_utest.dir/test_dotu.c.o utest/CMakeFiles/openblas_utest.dir/test_potrs.c.o utest/CMakeFiles/openblas_utest.dir/test_kernel_regress.c.o -o utest/openblas_utest -Wl,-rpath,/.../openblas/build/lib lib/libopenblas.so.0.3 -lm clang version 18.0.0 (g...@github.com:llvm/llvm-project.git 17feb330aab39c6c0c21ee9b02efb484dfb2261e) Target: aarch64-unknown-linux-gnu Thread model: posix InstalledDir: /.../llvm/trunk/bin Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/11 Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/12 Selected GCC installation: /usr/lib/gcc/aarch64-linux-gnu/12 Candidate multilib: .;@m64 Selected multilib: .;@m64 Found CUDA installation: /usr/local/cuda, version "/usr/bin/ld" -EL -z relro --hash-style=gnu --eh-frame-hdr -m aarch64linux -pie -dynamic-linker /lib/ld-linux-aarch64.so.1 -o utest/openblas_utest /lib/aarch64-linux-gnu/Scrt1.o /lib/aarch64-linux-gnu/crti.o /usr/lib/gcc/aarch64-linux-gnu/12/crtbeginS.o -L/.../llvm/trunk/lib/clang/18/lib/aarch64-unknown-linux-gnu -L/usr/lib/gcc/aarch64-linux-gnu/12 -L/lib/aarch64-linux-gnu -L/usr/lib/aarch64-linux-gnu -L/lib -L/usr/lib -L/.../llvm/trunk/lib utest/CMakeFiles/openblas_utest.dir/utest_main.c.o utest/CMakeFiles/openblas_utest.dir/test_min.c.o utest/CMakeFiles/openblas_utest.dir/test_amax.c.o utest/CMakeFiles/openblas_utest.dir/test_ismin.c.o utest/CMakeFiles/openblas_utest.dir/test_rotmg.c.o utest/CMakeFiles/openblas_utest.dir/test_rot.c.o utest/CMakeFiles/openblas_utest.dir/test_axpy.c.o utest/CMakeFiles/openblas_utest.dir/test_dsdot.c.o utest/CMakeFiles/openblas_utest.dir/test_dnrm2.c.o utest/CMakeFiles/openblas_utest.dir/test_swap.c.o utest/CMakeFiles/openblas_utest.dir/test_dotu.c.o utest/CMakeFiles/openblas_utest.dir/test_potrs.c.o utest/CMakeFiles/openblas_utest.dir/test_kernel_regress.c.o -rpath /.../openblas/build/lib lib/libopenblas.so.0.3 -lm -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/aarch64-linux-gnu/12/crtendS.o /lib/aarch64-linux-gnu/crtn.o /usr/bin/ld: lib/libopenblas.so.0.3: undefined reference to `_QQEnvironmentDefaults' /usr/bin/ld: lib/libopenblas.so.0.3: undefined reference to `_QQmain' ``` https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: Ok, got it! I'm fione with reverting this patch and seeking a better solution. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
tblah wrote:
Simple reproducer:
main.c
```
int main(void) {
// call fortran code
return 0;
}
```
fortran.f90
```
function pow(a, b)
real :: a, b, pow
pow = a ** b
end function
```
```
clang -c main.c -o main.o
flang-now -c fortran.f90 -o fortran.o
flang-new fortran.o main.o -o out
```
This can be built with gfortran
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
tblah wrote: Yes they are mixed source projects. They worked previously because the main function from `Frotran_main` was silently ignored. I don't know if this was accidental or not, but it tended to do the right thing for mixed source projects because if the user intended to use `Fortran_main`, they would not define `main()` elsewhere, but if they did not, there would be another definition of `main()`. I think the simplest solution would be to add a flag controlling whether to link `Fortran_main`. @DavidTruby suggested only linking `Fortran_main` if the fortran source contains a `program` statement. One could build a mixed source project now by linking using `clang` and manually specifying to link the rest of the fortran runtime library. I don't think this is a good option because it is not convenient to change spec makefiles and I don't think expecting users to find the right fortran runtime linker invocation is a good experience (a flag just for `Fortran_main` is better because one could see "error: multiple definitions of main()" ... "ahh I need the flag to get rid of the automatic definition of `main()`"). https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
banach-space wrote: > Since this patch, I can no longer build spec2006 gromacs and calculix because > they supply their own main function in a C file, then link using flang-new: > leading to another definition of `main()` from the runtime. Do I need to use > a special flag to avoid linking to libFortran_main? How about `flang-new -c file.90`? Which driver does the linking at the end? TBH, sounds like you are building a mixed-source project. This should be supported by the Clang/Flang duo, but we haven't really tested that. It's possible that it "accidentally" worked, but this patch made the driver stricter and now we know that something is missing. What's the compiler invoc? https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
tblah wrote: Since this patch, I can no longer build spec2006 gromacs and calculix because they supply their own main function in a C file, then link using flang-new: leading to another definition of `main()` from the runtime. Do I need to use a special flag to avoid linking to libFortran_main? https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
psteinfeld wrote: > @mjklemm, after this change was integrated, the test Driver/ctofortran no > longer succeeds. This test gets run when you call `make check-flang`. I'm not > sure why the pre- and post-build checks did not run check-flang. I've submitted #73738 to fix this. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
psteinfeld wrote: @mjklemm, after this change was integrated, the test Driver/ctofortran no longer succeeds. This test gets run when you call `make check-flang`. I'm not sure why the pre- and post-build checks did not run check-flang. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: Thanks all for your reviews and the all the feedback you have provided! Much appreciated! https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/kparzysz closed https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/16] Let the linker fail on multiple definitions of main()
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 1f31c6395206ee8..740ae71177b2c3a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -982,7 +982,20 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// 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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang-new new
+// line will not sucessfully complete, unless the user correctly specified
+// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
+// -Wl,--no-whole-archive).
+CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
+CmdArgs.push_back("--no-whole-archive");
+
+// Perform regular linkage of the remaining runtime libraries.
CmdArgs.push_back("-lFortranRuntime");
CmdArgs.push_back("-lFortranDecimal");
}
@@ -993,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
+ // this in the future. In particular, on some platforms, we may need to useq
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 0d652282f4dbed2dde11df53ead3e6c8b6856bed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 15:18:51 +0100
Subject: [PATCH 02/16] Improve comments and remove accidental typo
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 740ae71177b2c3a..464a87737de062c 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -987,10 +987,10 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// the linker.
//
// We are using this --whole-archive/--no-whole-archive bracket w/o
-// any further checks, because -Wl,--whole-archive at the flang-new new
-// line will not sucessfully complete, unless the user correctly specified
-// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
-// -Wl,--no-whole-archive).
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("--no-whole-archive");
@@ -1006,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to useq
+ // this in the future. In particular, on some platforms, we may need to use
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 39612e237cb815cf4ea0120027783d35304bcb6b Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 20:26:02 +0100
Subject: [PATCH 03/16] Correct link line test for flang-new (for Linux)
---
flang/test/Driver/linker-flags.f90 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/test/Driver/linker-flags.f90
b/flang/test/Driver/linker-flags.f90
index 85c4d60b3f09862..ea91946316cfaa6 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -28,7 +28,7 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "-lFortran
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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 NeedWholeArchive = true;
+auto * Arg = Args.getLastArg(options::OPT_Wl_COMMA);
mjklemm wrote:
Should be fixed, too.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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 NeedWholeArchive = true;
+auto * Arg = Args.getLastArg(options::OPT_Wl_COMMA);
+for (StringRef ArgValue : llvm::reverse(Arg->getValues())) {
+ if (ArgValue == "--whole-archive") {
+NeedWholeArchive = false;
+break;
+ }
mjklemm wrote:
Should be fixed now.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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 NeedWholeArchive = true;
+auto * Arg = Args.getLastArg(options::OPT_Wl_COMMA);
kparzysz wrote:
This can be `nullptr` if the -Wl option wasn't present, so we need to check for
that.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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 NeedWholeArchive = true;
+auto * Arg = Args.getLastArg(options::OPT_Wl_COMMA);
+for (StringRef ArgValue : llvm::reverse(Arg->getValues())) {
+ if (ArgValue == "--whole-archive") {
+NeedWholeArchive = false;
+break;
+ }
kparzysz wrote:
This needs an `else if (ArgValue == "-no-whole-archive") break`, or otherwise
we'll end up ignoring it.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/14] Let the linker fail on multiple definitions of main()
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 1f31c6395206ee8..740ae71177b2c3a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -982,7 +982,20 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// 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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang-new new
+// line will not sucessfully complete, unless the user correctly specified
+// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
+// -Wl,--no-whole-archive).
+CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
+CmdArgs.push_back("--no-whole-archive");
+
+// Perform regular linkage of the remaining runtime libraries.
CmdArgs.push_back("-lFortranRuntime");
CmdArgs.push_back("-lFortranDecimal");
}
@@ -993,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
+ // this in the future. In particular, on some platforms, we may need to useq
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 0d652282f4dbed2dde11df53ead3e6c8b6856bed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 15:18:51 +0100
Subject: [PATCH 02/14] Improve comments and remove accidental typo
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 740ae71177b2c3a..464a87737de062c 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -987,10 +987,10 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// the linker.
//
// We are using this --whole-archive/--no-whole-archive bracket w/o
-// any further checks, because -Wl,--whole-archive at the flang-new new
-// line will not sucessfully complete, unless the user correctly specified
-// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
-// -Wl,--no-whole-archive).
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("--no-whole-archive");
@@ -1006,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to useq
+ // this in the future. In particular, on some platforms, we may need to use
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 39612e237cb815cf4ea0120027783d35304bcb6b Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 20:26:02 +0100
Subject: [PATCH 03/14] Correct link line test for flang-new (for Linux)
---
flang/test/Driver/linker-flags.f90 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/test/Driver/linker-flags.f90
b/flang/test/Driver/linker-flags.f90
index 85c4d60b3f09862..ea91946316cfaa6 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -28,7 +28,7 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "-lFortran
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,63 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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)
mjklemm wrote:
Thanks for the great suggestion!
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: Folks, I have made another attempt to improve this patch. @kparzysz with your feedback in mind, I have now added a check if `--whole-archive` is active for some reason. If so, flang will not add it to the link line again. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/13] Let the linker fail on multiple definitions of main()
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 1f31c6395206ee8..740ae71177b2c3a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -982,7 +982,20 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// 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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang-new new
+// line will not sucessfully complete, unless the user correctly specified
+// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
+// -Wl,--no-whole-archive).
+CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
+CmdArgs.push_back("--no-whole-archive");
+
+// Perform regular linkage of the remaining runtime libraries.
CmdArgs.push_back("-lFortranRuntime");
CmdArgs.push_back("-lFortranDecimal");
}
@@ -993,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
+ // this in the future. In particular, on some platforms, we may need to useq
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 0d652282f4dbed2dde11df53ead3e6c8b6856bed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 15:18:51 +0100
Subject: [PATCH 02/13] Improve comments and remove accidental typo
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 740ae71177b2c3a..464a87737de062c 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -987,10 +987,10 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// the linker.
//
// We are using this --whole-archive/--no-whole-archive bracket w/o
-// any further checks, because -Wl,--whole-archive at the flang-new new
-// line will not sucessfully complete, unless the user correctly specified
-// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
-// -Wl,--no-whole-archive).
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("--no-whole-archive");
@@ -1006,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to useq
+ // this in the future. In particular, on some platforms, we may need to use
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 39612e237cb815cf4ea0120027783d35304bcb6b Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 20:26:02 +0100
Subject: [PATCH 03/13] Correct link line test for flang-new (for Linux)
---
flang/test/Driver/linker-flags.f90 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/test/Driver/linker-flags.f90
b/flang/test/Driver/linker-flags.f90
index 85c4d60b3f09862..ea91946316cfaa6 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -28,7 +28,7 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "-lFortran
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/kparzysz approved this pull request. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
kparzysz wrote:
Fine with me.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
mjklemm wrote:
Guaranteed might be too much. :-) But at least it's common sense that
--whole-archive will have a good chance of breaking any link line, unless the
user closes it with --no-whole-archive right after the desired library.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
kparzysz wrote:
What I meant is that it's not clear that we can actually skip this check and
get away with it forever. Is that guaranteed?
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs,
const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
kparzysz wrote:
This may be true now, but we don't know if it will always be true.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote:
> I think for Windows the easy thing to do here is just to add
> `/WHOLEARCHIVE:...` here anyway, using the same logic as in
> processVSRuntimeLibrary(); E.g
>
> ```
> void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
> //need to add args here so we can search it
> llvm::opt::ArgStringList &CmdArgs) {
> if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) {
> // non-windows stuff
> } else {
> 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");
> ///etc
> }
> ```
>
> I haven't actually tested this code but it maybe gives an idea of what needs
> doing?
Yeah, that's pretty much what I was doing before the code was refactored with
this commit:
```
commit 0bc7cd4d51226344a54da5929d87184730e73e83
Author: David Truby
Date: Thu Nov 23 14:19:57 2023 +
[flang] Add runtimes using --dependent-lib on MSVC targets (#72519)
```
So, I'd have to replicate part of the code that was removed from
`lib/Driver/ToolChains/CommonArgs.cpp` to get `/WHOLEARCHIVE` back into the
link line?
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
DavidTruby wrote:
I think for Windows the easy thing to do here is just to add
`/WHOLEARCHIVE:...` here anyway, using the same logic as in
processVSRuntimeLibrary(); E.g
```
void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
//need to add args here so we can search it
llvm::opt::ArgStringList &CmdArgs) {
if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) {
// non-windows stuff
} else {
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");
///etc
}
```
I haven't actually tested this code but it maybe gives an idea of what needs
doing?
The "correct" thing to do would probably be to add the /WHOLEARCHIVE directive
as a linker directive in the object file (like we do for
/DEFAULTLIB:Fortran_main) but that would require a bit more hooking up to pass
that all the way through to creating an `llvm.linker_options` MLIR operation. I
think the above should probably suffice at least for now.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: FYI: Rebased and resolved conflict flagged by GitHub. Alas, this means that I have lost the change to also make the linker fail on Windows. I've sent a request to the authors of the code to perform linking on Windows to help me figure out what to do. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/11] Let the linker fail on multiple definitions of main()
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 1f31c6395206ee8..740ae71177b2c3a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -982,7 +982,20 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// 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()) {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang-new new
+// line will not sucessfully complete, unless the user correctly specified
+// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
+// -Wl,--no-whole-archive).
+CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
+CmdArgs.push_back("--no-whole-archive");
+
+// Perform regular linkage of the remaining runtime libraries.
CmdArgs.push_back("-lFortranRuntime");
CmdArgs.push_back("-lFortranDecimal");
}
@@ -993,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
+ // this in the future. In particular, on some platforms, we may need to useq
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 0d652282f4dbed2dde11df53ead3e6c8b6856bed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 15:18:51 +0100
Subject: [PATCH 02/11] Improve comments and remove accidental typo
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 740ae71177b2c3a..464a87737de062c 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -987,10 +987,10 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
// the linker.
//
// We are using this --whole-archive/--no-whole-archive bracket w/o
-// any further checks, because -Wl,--whole-archive at the flang-new new
-// line will not sucessfully complete, unless the user correctly specified
-// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
-// -Wl,--no-whole-archive).
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("--no-whole-archive");
@@ -1006,7 +1006,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to useq
+ // this in the future. In particular, on some platforms, we may need to use
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 39612e237cb815cf4ea0120027783d35304bcb6b Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 20:26:02 +0100
Subject: [PATCH 03/11] Correct link line test for flang-new (for Linux)
---
flang/test/Driver/linker-flags.f90 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/test/Driver/linker-flags.f90
b/flang/test/Driver/linker-flags.f90
index 85c4d60b3f09862..ea91946316cfaa6 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -28,7 +28,7 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "-lFortran
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
banach-space wrote: > > LGTM, thank you for taking care of this 🙏🏻 > > Dare I ask - what's "dupes"? I only found > > [dupe](https://dictionary.cambridge.org/dictionary/english/dupe). Also, > > please wait for @kiranchandramohan to approve before merging this :) > > I used "dupes" in the sense of being fooled. I can of course still change the > name to something like "main_linktime_duplicate.f90" or the likes. Right, "dupes" wasn't that obvious to me :) I would probably go for your other suggestion or just plain "duplicate_main.f90". Either would be a bit more descriptive IMHO 🙏🏻 . Thanks again for addressing my comments and for this contribution! https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,15 @@ +! UNSUPPORTED: system-windows + +! RUN: %flang -x ir -o %t.c-object -c %S/Inputs/main_dupes.ll +! RUN: %flang -o %t -c %s +! RUN: not %flang -o %t.exe %t %t.c-object 2>&1 banach-space wrote: That was just a nit - I am happy if you keep things as is 👍🏻 ! https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,15 @@ +! UNSUPPORTED: system-windows + +! RUN: %flang -x ir -o %t.c-object -c %S/Inputs/main_dupes.ll +! RUN: %flang -o %t -c %s +! RUN: not %flang -o %t.exe %t %t.c-object 2>&1 mjklemm wrote: I'd actually prefer to have a separate test for this, as a working test is any other test that actually produces an executable. If you insist :-), then I'd even go and change the test from testing for a duplicate of main to a general link time test that tests both a successful link and the failing one that I'm after. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > LGTM, thank you for taking care of this 🙏🏻 > > Dare I ask - what's "dupes"? I only found > [dupe](https://dictionary.cambridge.org/dictionary/english/dupe). Also, > please wait for @kiranchandramohan to approve before merging this :) I used "dupes" in the sense of being fooled. I can of course still change the name to something like "main_linktime_duplicate.f90" or the likes. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,15 @@ +! UNSUPPORTED: system-windows + +! RUN: %flang -x ir -o %t.c-object -c %S/Inputs/main_dupes.ll +! RUN: %flang -o %t -c %s +! RUN: not %flang -o %t.exe %t %t.c-object 2>&1 banach-space wrote: [nit] You could add a "valid"/"working" compilation line to contrast it with the broken one. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -122,6 +122,7 @@
# the build directory holding that tool.
tools = [
ToolSubst("%flang", command=FindTool("flang-new"), unresolved="fatal"),
+ToolSubst("%clang", command=FindTool("clang"), unresolved="fatal"),
mjklemm wrote:
I have removed this in favor of compiling a LL file instead, like suggest below.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff e5cc3da6a9077548f613eee3aacc5e7b017c81f3
3b0090997023b1b6392bc23d386ace7c7cb796ce --
flang/test/Driver/Inputs/main_dupes.c clang/lib/Driver/ToolChains/CommonArgs.cpp
``
View the diff from clang-format here.
``diff
diff --git a/flang/test/Driver/Inputs/main_dupes.c
b/flang/test/Driver/Inputs/main_dupes.c
index baae10f94e..dd318c0816 100644
--- a/flang/test/Driver/Inputs/main_dupes.c
+++ b/flang/test/Driver/Inputs/main_dupes.c
@@ -1,8 +1,8 @@
#include
-int main(int argc, char * argv[]) {
-// Irrelevant what to do in here.
-// Test is supposed to fail at link time.
-printf("Hello from C [%s]\n", __FUNCTION__);
-return 0;
+int main(int argc, char *argv[]) {
+ // Irrelevant what to do in here.
+ // Test is supposed to fail at link time.
+ printf("Hello from C [%s]\n", __FUNCTION__);
+ return 0;
}
``
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: Done. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From ba38aec7ac04c63fd5167908fe7f91d6ac7bceed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 1/6] Let the linker fail on multiple definitions of main()
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 5d2cd1959b06925..30c249d05677ce5 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1018,7 +1018,20 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
const ArgList &Args,
break;
}
} else {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang-new new
+// line will not sucessfully complete, unless the user correctly specified
+// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
+// -Wl,--no-whole-archive).
+CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
+CmdArgs.push_back("--no-whole-archive");
+
+// Perform regular linkage of the remaining runtime libraries.
CmdArgs.push_back("-lFortranRuntime");
CmdArgs.push_back("-lFortranDecimal");
}
@@ -1029,7 +1042,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
+ // this in the future. In particular, on some platforms, we may need to useq
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 7d1180b11ed02cedf1c9fea56bf2ff329274c066 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 15:18:51 +0100
Subject: [PATCH 2/6] Improve comments and remove accidental typo
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 30c249d05677ce5..12e3ce184898250 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1023,10 +1023,10 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
const ArgList &Args,
// the linker.
//
// We are using this --whole-archive/--no-whole-archive bracket w/o
-// any further checks, because -Wl,--whole-archive at the flang-new new
-// line will not sucessfully complete, unless the user correctly specified
-// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
-// -Wl,--no-whole-archive).
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("--no-whole-archive");
@@ -1042,7 +1042,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to useq
+ // this in the future. In particular, on some platforms, we may need to use
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 30dab7ebddf1de4836fc3d532fc33b4a7e58837d Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 20:26:02 +0100
Subject: [PATCH 3/6] Correct link line test for flang-new (for Linux)
---
flang/test/Driver/linker-flags.f90 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/test/Driver/linker-flags.f90
b/flang/test/Driver/linker-flags.f90
index a1417057d4da068..55b13952db43c17 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -31,7 +31,7 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "-lFortran_main" "-lFortranRuntime" "-lFortranDecimal" "-lm"
+! UNIX-SAME: "--whole-archive" "-lFortran_main" "--no-whole-archive"
"-lFortranRuntime"
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > Would it be possible to test this? I have added a simple test. It checks that the linker indeed fails, but does not check the actual linker error message. PLease let me know if that's desired. I could then provide the error messages for LD and LLD. For other environments, I'd have to rely on the community to help me getting the proper linker error messages for the test. https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
banach-space wrote: As this is a "test input" rather than a "test file", could you move it to "flang/test/Driver/Inputs"? https://github.com/llvm/llvm-project/pull/73124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -122,6 +122,7 @@
# the build directory holding that tool.
tools = [
ToolSubst("%flang", command=FindTool("flang-new"), unresolved="fatal"),
+ToolSubst("%clang", command=FindTool("clang"), unresolved="fatal"),
mjklemm wrote:
@banach-space I had to add this, as I need both flang and clang to compile the
test. This could move to a different PR if needed. Please advise!
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From ba38aec7ac04c63fd5167908fe7f91d6ac7bceed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 1/5] Let the linker fail on multiple definitions of main()
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 5d2cd1959b06925..30c249d05677ce5 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1018,7 +1018,20 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
const ArgList &Args,
break;
}
} else {
+// --whole-archive 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.
+//
+// We are using this --whole-archive/--no-whole-archive bracket w/o
+// any further checks, because -Wl,--whole-archive at the flang-new new
+// line will not sucessfully complete, unless the user correctly specified
+// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
+// -Wl,--no-whole-archive).
+CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
+CmdArgs.push_back("--no-whole-archive");
+
+// Perform regular linkage of the remaining runtime libraries.
CmdArgs.push_back("-lFortranRuntime");
CmdArgs.push_back("-lFortranDecimal");
}
@@ -1029,7 +1042,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
+ // this in the future. In particular, on some platforms, we may need to useq
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 7d1180b11ed02cedf1c9fea56bf2ff329274c066 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 15:18:51 +0100
Subject: [PATCH 2/5] Improve comments and remove accidental typo
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 30c249d05677ce5..12e3ce184898250 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1023,10 +1023,10 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC,
const ArgList &Args,
// the linker.
//
// We are using this --whole-archive/--no-whole-archive bracket w/o
-// any further checks, because -Wl,--whole-archive at the flang-new new
-// line will not sucessfully complete, unless the user correctly specified
-// -Wl,--no-whole-archive (e.g., -Wl,--whole-archive -ldummy
-// -Wl,--no-whole-archive).
+// any further checks, because -Wl,--whole-archive at the flang
+// driver's link line will not sucessfully complete, unless the user
+// correctly specified -Wl,--whole-archive/-Wl,--no-whole-archive
+// (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive).
CmdArgs.push_back("--whole-archive");
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("--no-whole-archive");
@@ -1042,7 +1042,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain
&TC,
ArgStringList &CmdArgs) {
// Default to the /../lib directory. This works fine on the
// platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to useq
+ // this in the future. In particular, on some platforms, we may need to use
// lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(TC.getDriver().Dir);
>From 30dab7ebddf1de4836fc3d532fc33b4a7e58837d Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 20:26:02 +0100
Subject: [PATCH 3/5] Correct link line test for flang-new (for Linux)
---
flang/test/Driver/linker-flags.f90 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/test/Driver/linker-flags.f90
b/flang/test/Driver/linker-flags.f90
index a1417057d4da068..55b13952db43c17 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -31,7 +31,7 @@
! executable and may find the GNU linker from MinGW or Cygwin.
! UNIX-LABEL: "{{.*}}ld{{(\.exe)?}}"
! UNIX-SAME: "[[object_file]]"
-! UNIX-SAME: "-lFortran_main" "-lFortranRuntime" "-lFortranDecimal" "-lm"
+! UNIX-SAME: "--whole-archive" "-lFortran_main" "--no-whole-archive"
"-lFortranRuntime"
