[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
mjklemm wrote: @DavidTruby If you are OK with the way I handled MSVC, please approve and I will merge the PR (or change it if you want some changes to be made). https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm 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' > ``` Thanks for the report! Can you please tell me how OpenBLAS was built? I'm trying to replicate this, but I do not see a reference to `_QQmain` or the likes in the OpenBLAS library that I build on x86. 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] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/7] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d..aa26344f67b31 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/7] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e9..2899f07cb7490 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/7] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490..f0e3df2a63ae9 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Dr
[clang] [flang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/8] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d..aa26344f67b31 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/8] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e9..2899f07cb7490 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/8] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490..f0e3df2a63ae9 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Dr
[clang] [flang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
@@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + mjklemm wrote: @banach-space ``` let Visibility = [FlangOption] in { def no_fortran_main : Flag<["-"], "fno-fortran-main">, Visibility<[FlangOption]>, Group, HelpText<"Don't link in Fortran main">; } // let Visibility = [ FlangOption ] ``` Was that the suggested edit? I guess `-J` should not be be modified as part of the edit, right? https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
mjklemm wrote: > I know that it's extra work, but it's also super useful bit of documentation. > And I would only rarely try to track the history beyond commit messages > (there's just too much otherwise). I can absolutely do that. I'll craft some wording for the rationale behind that change and add it to the PR. https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm edited https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Rename `flang-new` as `flang` (PR #74377)
mjklemm wrote: I have a Windows test machine. How can I help? https://github.com/llvm/llvm-project/pull/74377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm edited https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/9] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d6..aa26344f67b313 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/9] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e94..2899f07cb7490c 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/9] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490c..f0e3df2a63ae98 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
@@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; mjklemm wrote: Done, with a slightly different wording https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From e0784bd3a6103fe6852ecc67fb01a4a8da1cf59a Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/9] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index b959fd20fe413..057f1f4b90955 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6354,6 +6354,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 5c2f898be0d16fdecd44fbcf94cb0513e9a75bb2 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/9] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 51b336216c565..82ed3e53d9960 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From a87bdd40e2f4e707a47a185b49a93ae84c809bda Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/9] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 82ed3e53d9960..93877b75beaf7 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Dr
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm closed https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: @tblah @rj-jesus https://github.com/llvm/llvm-project/pull/74139 has landed. Can you please see if `-fno-fortran-main` helps to resolve your bugs? I will write up a blog article about this and publish it. Maybe it would be worth documenting this as a question on StackOverflow, 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][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm created https://github.com/llvm/llvm-project/pull/73124 The flang driver was silently ignoring the `main()` function in `Fortran_main.a` for entry into the Fortran program unit if an external `main()` as supplied (e.g., via cross-language linkage with Fortran and C/C++). This PR fixes this by making sure that the linker always pulls in the `main()` definition from `Fortran_main.a` and consequently fails due to multiple definitions of the same symbol if another object file also has a definition of `main()`. >From ba38aec7ac04c63fd5167908fe7f91d6ac7bceed Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 22 Nov 2023 14:22:20 +0100 Subject: [PATCH] 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); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [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/2] 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/2] 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); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -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 mjklemm wrote: Yes, indeed. Sorry about that! I have also found typos in my comment. Fixed these, 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][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > Would it be possible to test this? Sure thing! That's the next thing I can look at. I'd like to probe though if the general solution is acceptable. 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][driver] Don't use -whole-archive on Darwin (PR #75393)
mjklemm wrote: > > LGTM. Worked fine on my machine. > > > > > > NOTE: tested by replacing `CommonArgs.cpp` in main, to avoid conflicts. > > > > Thanks for checking and apologies for the merge conflict - I thought that I > was up to date :( I've just rebased and force-pushed. > > > > I will be landing this today if there are no further comments 🙏🏻 This patch seems to hide the original problem of the compiler not erroring out when multiple definitions of main happen. T think we should rather inject the proper linker equivalent of --whole-archive on Darwin, too. I'd be OK to land this patch for now, if we agree to have a follow up PR to (re-)instantiate the error handling also for Darwin. https://github.com/llvm/llvm-project/pull/75393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang][driver] Don't use -whole-archive on Darwin (PR #75393)
mjklemm wrote: Let's get this in. I agree with the assessment by @banach-space. The PR looks good to me. https://github.com/llvm/llvm-project/pull/75393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][nfc] Refactor linker invocation logic (PR #75534)
https://github.com/mjklemm edited 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)
@@ -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) { 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)
@@ -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) { 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][nfc] Refactor linker invocation logic (PR #75648)
https://github.com/mjklemm approved this pull request. Looks good to me. Did touch testing with my reproducers and it worked as I would expect it. https://github.com/llvm/llvm-project/pull/75648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
https://github.com/mjklemm created https://github.com/llvm/llvm-project/pull/75816 At present, when building static or shared libraries, Flang adds `-lFortran_main.a` (or `/WHOLEARCHIVE:Fortran.*.lib` pon Windows) to the link line. This leads to the problem that `_QQmain` and `_QQEnvironmentDefaults` (as of the time of this PR) are symbols marked as used, while `main` is being defined. This should not happen and this PR fixes this by detecting if `-shared` or `-static` is used on the Flang command line and removing the static `Fortran_main` library. >From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Sat, 16 Dec 2023 20:15:17 +0100 Subject: [PATCH 1/2] Remove -lFortran_main from the link line when -shared is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ 1 file changed, 22 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 45901ee7157f77..5d525d3794ad1d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +static bool isSharedLinkage(const ArgList &Args) { + bool FoundSharedFlag = false; + for (auto *Arg : Args.filtered(options::OPT_shared)) { +if (Arg) { + FoundSharedFlag = true; +} + } + + return FoundSharedFlag; +} + /// Add Fortran runtime libs for MSVC static void addFortranRuntimeLibsMSVC(const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { @@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, // Add FortranMain runtime lib static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { + // 0. Shared-library linkage + // If we are attempting to link a shared library, we should not add + // -lFortran_main.a to the link line, as the `main` symbol is not + // required for a shared library and should also be provided by one + // of the translation units of the code that this shared library + // will be linked against eventually. + if (isSharedLinkage(Args)) { +printf("MK: --> shared linkage, do not add -lFortranMain\n"); +return; + } + // 1. MSVC if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { addFortranRuntimeLibsMSVC(Args, CmdArgs); >From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Mon, 18 Dec 2023 11:27:59 +0100 Subject: [PATCH 2/2] Update dynamic_linker.f90 test and clean up a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ flang/test/Driver/dynamic-linker.f90 | 8 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 5d525d3794ad1d..05ebd42829c95d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +/// Determine if driver is invoked to create a shared object library (-static) static bool isSharedLinkage(const ArgList &Args) { - bool FoundSharedFlag = false; - for (auto *Arg : Args.filtered(options::OPT_shared)) { -if (Arg) { - FoundSharedFlag = true; -} - } + return Args.hasArg(options::OPT_shared); +} - return FoundSharedFlag; +/// Determine if driver is invoked to create a static object library (-shared) +static bool isStaticLinkage(const ArgList &Args) { + return Args.hasArg(options::OPT_static); } /// Add Fortran runtime libs for MSVC @@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // 0. Shared-library linkage - // If we are attempting to link a shared library, we should not add + // If we are attempting to link a library, we should not add // -lFortran_main.a to the link line, as the `main` symbol is not - // required for a shared library and should also be provided by one - // of the translation units of the code that this shared library + // required for a library and should also be provided by one of + // the translation units of the code that this shared library // will be linked against eventually. - if (isSharedLinkage(Args)) { -printf("MK: --> shared linkage, do not add -lFortranMain\n"); + if (isSharedLinkage(Args) || isStaticLinkage(Args)) { return; } diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90 index df119c22a2ea51..af07e2483f93fa 100644 --- a/flang/test/Driver/dynamic-linker.f90 +++ b/flang/test/Driver/dynamic-linker.f90
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
mjklemm wrote: > Thanks! Mostly looks good, but I have a question: > > > This leads to the problem that _QQmain and _QQEnvironmentDefaults (as of > > the time of this PR) are symbols marked as used, while main is being > > defined. > > Sorry for being pedantic, but not sure I follow. `FortranMain` defines > `main`, which calls `_QQmain`. However, the latter wouldn't be available in a > library, right? Yes, that's correct. The `main` symbol calls `_QQmain`, which happens to be the entry point for the Fortran program unit. > Also, not sure what defines `_QQEnvironmentDefaults ` and what exactly makes > it problematic? The `main` etrypoint also calls `_QQEnvironmentDefaults` to treat `argc`, `argv`, etc. So, when `main` is included in the shared object, so will be those symbols are used symbols, which makes no sense. > Btw, would you mind adding a note to the driver docs? Sure, I was planning to do this for the previous PR already but got distracted. Can you point me to the right place to add this? Thanks! https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
mjklemm wrote: > > Also, not sure what defines `_QQEnvironmentDefaults ` and what exactly > > makes it problematic? > > The `main` entrypoint also calls `_QQEnvironmentDefaults` to treat `argc`, > `argv`, etc. So, when `main` is included in the shared object, so will be > those symbols are used symbols, which makes no sense. I need to correct myself: _QQEnvironmentDefaults is not a function that is called, but rather an external file-scope variable that creates that symbol. The symbol used to initialize that is `_FortranAProgramStart`. Plus, there's `_FortranAProgramEndStatement` that also used by `Fortran_main.a`. So, lot's of symbols that should not be in a dynamic library for no reason. :-) https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > I see. So Fortran and C interoperability of F2003/F2008 is not supported yet > in Flang? It's rather untested and I'm working hard to get it fixed, so that we have well-defined behavior that is similar to other Fortran compilers like ifx and gfortran. > 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. Please share as many of these reproducers as you can, so that I can debug the regression and fix it. ``` 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) ``` This error message is a strong indication of potentially two things: - Both Fortran and C/C++ provide the `main` entry point (Fortran via `Fortran_main.a` and C/C++ via a `main` function). Since this led to some weird behavior in the past, this situation now leads to an error message. - On C/C++ provides the `main` function. In this case, please pass `-fno-fortran-main` when using `flang-new` link command line. This will remove the `main` symbol coming from `Fortran_main.a`, because it's not needed. 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] Remove Fortain_main static library from linking stages (PR #75816)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/75816 >From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Sat, 16 Dec 2023 20:15:17 +0100 Subject: [PATCH 1/3] Remove -lFortran_main from the link line when -shared is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ 1 file changed, 22 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 45901ee7157f77..5d525d3794ad1d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +static bool isSharedLinkage(const ArgList &Args) { + bool FoundSharedFlag = false; + for (auto *Arg : Args.filtered(options::OPT_shared)) { +if (Arg) { + FoundSharedFlag = true; +} + } + + return FoundSharedFlag; +} + /// Add Fortran runtime libs for MSVC static void addFortranRuntimeLibsMSVC(const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { @@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, // Add FortranMain runtime lib static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { + // 0. Shared-library linkage + // If we are attempting to link a shared library, we should not add + // -lFortran_main.a to the link line, as the `main` symbol is not + // required for a shared library and should also be provided by one + // of the translation units of the code that this shared library + // will be linked against eventually. + if (isSharedLinkage(Args)) { +printf("MK: --> shared linkage, do not add -lFortranMain\n"); +return; + } + // 1. MSVC if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { addFortranRuntimeLibsMSVC(Args, CmdArgs); >From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Mon, 18 Dec 2023 11:27:59 +0100 Subject: [PATCH 2/3] Update dynamic_linker.f90 test and clean up a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ flang/test/Driver/dynamic-linker.f90 | 8 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 5d525d3794ad1d..05ebd42829c95d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +/// Determine if driver is invoked to create a shared object library (-static) static bool isSharedLinkage(const ArgList &Args) { - bool FoundSharedFlag = false; - for (auto *Arg : Args.filtered(options::OPT_shared)) { -if (Arg) { - FoundSharedFlag = true; -} - } + return Args.hasArg(options::OPT_shared); +} - return FoundSharedFlag; +/// Determine if driver is invoked to create a static object library (-shared) +static bool isStaticLinkage(const ArgList &Args) { + return Args.hasArg(options::OPT_static); } /// Add Fortran runtime libs for MSVC @@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // 0. Shared-library linkage - // If we are attempting to link a shared library, we should not add + // If we are attempting to link a library, we should not add // -lFortran_main.a to the link line, as the `main` symbol is not - // required for a shared library and should also be provided by one - // of the translation units of the code that this shared library + // required for a library and should also be provided by one of + // the translation units of the code that this shared library // will be linked against eventually. - if (isSharedLinkage(Args)) { -printf("MK: --> shared linkage, do not add -lFortranMain\n"); + if (isSharedLinkage(Args) || isStaticLinkage(Args)) { return; } diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90 index df119c22a2ea51..af07e2483f93fa 100644 --- a/flang/test/Driver/dynamic-linker.f90 +++ b/flang/test/Driver/dynamic-linker.f90 @@ -3,18 +3,22 @@ ! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s +! RUN: --check-prefixes=GNU-LINKER-OPTIONS \ +! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s ! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s +! RUN:
[clang] [flang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/75816 >From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Sat, 16 Dec 2023 20:15:17 +0100 Subject: [PATCH 1/4] Remove -lFortran_main from the link line when -shared is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ 1 file changed, 22 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 45901ee7157f77..5d525d3794ad1d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +static bool isSharedLinkage(const ArgList &Args) { + bool FoundSharedFlag = false; + for (auto *Arg : Args.filtered(options::OPT_shared)) { +if (Arg) { + FoundSharedFlag = true; +} + } + + return FoundSharedFlag; +} + /// Add Fortran runtime libs for MSVC static void addFortranRuntimeLibsMSVC(const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { @@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, // Add FortranMain runtime lib static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { + // 0. Shared-library linkage + // If we are attempting to link a shared library, we should not add + // -lFortran_main.a to the link line, as the `main` symbol is not + // required for a shared library and should also be provided by one + // of the translation units of the code that this shared library + // will be linked against eventually. + if (isSharedLinkage(Args)) { +printf("MK: --> shared linkage, do not add -lFortranMain\n"); +return; + } + // 1. MSVC if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { addFortranRuntimeLibsMSVC(Args, CmdArgs); >From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Mon, 18 Dec 2023 11:27:59 +0100 Subject: [PATCH 2/4] Update dynamic_linker.f90 test and clean up a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ flang/test/Driver/dynamic-linker.f90 | 8 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 5d525d3794ad1d..05ebd42829c95d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +/// Determine if driver is invoked to create a shared object library (-static) static bool isSharedLinkage(const ArgList &Args) { - bool FoundSharedFlag = false; - for (auto *Arg : Args.filtered(options::OPT_shared)) { -if (Arg) { - FoundSharedFlag = true; -} - } + return Args.hasArg(options::OPT_shared); +} - return FoundSharedFlag; +/// Determine if driver is invoked to create a static object library (-shared) +static bool isStaticLinkage(const ArgList &Args) { + return Args.hasArg(options::OPT_static); } /// Add Fortran runtime libs for MSVC @@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // 0. Shared-library linkage - // If we are attempting to link a shared library, we should not add + // If we are attempting to link a library, we should not add // -lFortran_main.a to the link line, as the `main` symbol is not - // required for a shared library and should also be provided by one - // of the translation units of the code that this shared library + // required for a library and should also be provided by one of + // the translation units of the code that this shared library // will be linked against eventually. - if (isSharedLinkage(Args)) { -printf("MK: --> shared linkage, do not add -lFortranMain\n"); + if (isSharedLinkage(Args) || isStaticLinkage(Args)) { return; } diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90 index df119c22a2ea51..af07e2483f93fa 100644 --- a/flang/test/Driver/dynamic-linker.f90 +++ b/flang/test/Driver/dynamic-linker.f90 @@ -3,18 +3,22 @@ ! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s +! RUN: --check-prefixes=GNU-LINKER-OPTIONS \ +! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s ! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s +! RUN:
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
mjklemm wrote: > I would probably create a section on building executables and just mention > that 3 runtime libs will be linked auto-magically. Unless somebody uses > `-fno-fortran-main` ;-) Make it as short or as long as you prefer - your > contribution is greatly appreciated 🙏🏻 :) If have written up a first draft. Please have a look and let me know what you think. https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > 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? It was always generated before, but the bug was hidden by the linker doing the right thing by incidence. I have fixed that so that such an error does no longer unnoticed. 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] Remove Fortain_main static library from linking stages (PR #75816)
mjklemm wrote: @banach-space How did you draw the pictures in the MD file that I'm changing? If you have some sort of source file, I can try to add a nice chart to the explanation, I'm adding. https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,40 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver +When used as a linker, Flang's frontend driver assembles the command line for an +external linker command (e.g., LLVM's `lld`) and invokes it to create the final +executable by linking static and shared libraries together with all the +translation units supplied as object files. + +By default, the Flang linker driver adds several libraries to the linker +invocation to make sure that all entrypoints for program start +(Fortran's program unit) and runtime routines can be resolved by the linker. +The libraries are: + +* `Fortran_main`: Provides the main entry point `main` that then invokes + `_QQmain` with the Fortran program unit. This library has a dependency to + the `FortranRuntime` library. +* `FortranRuntime`: Provides most of the Flang runtime library. +* `FortranDecimal`: Provides operations for decimal numbers. + +The default is that, when using Flang as the linker, one of the Fortran +translation units provides the program unit and therefore it is assumed that +Fortran is the main code part (calling into C/C++ routines via `BIND +(C)` interfaces). When composing the linker commandline, Flang uses +`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`, +Darwin: *not implemented yet*) to make sure that all for `Fortran_main` is +processed by the linker. This is done to issue a proper error message when +multiple definitions of `main` occur. This happens, for instance, when linking +a code that has a Fortran program unit with a C/C++ code that also defines a +`main` function. + +If the code is C/C++ based and invokes Fortran routines, either use Clang as the +linker driver (supplying `FortranRuntime` and/or `FortranDecimal` to the linker mjklemm wrote: Yes. Similar to what one would have to do if not using an MPI wrapper to compile an MPI program. I'll make this is a bit clearer. https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,40 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver +When used as a linker, Flang's frontend driver assembles the command line for an +external linker command (e.g., LLVM's `lld`) and invokes it to create the final +executable by linking static and shared libraries together with all the +translation units supplied as object files. + +By default, the Flang linker driver adds several libraries to the linker +invocation to make sure that all entrypoints for program start +(Fortran's program unit) and runtime routines can be resolved by the linker. +The libraries are: + +* `Fortran_main`: Provides the main entry point `main` that then invokes + `_QQmain` with the Fortran program unit. This library has a dependency to + the `FortranRuntime` library. +* `FortranRuntime`: Provides most of the Flang runtime library. +* `FortranDecimal`: Provides operations for decimal numbers. + +The default is that, when using Flang as the linker, one of the Fortran +translation units provides the program unit and therefore it is assumed that +Fortran is the main code part (calling into C/C++ routines via `BIND +(C)` interfaces). When composing the linker commandline, Flang uses +`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`, +Darwin: *not implemented yet*) to make sure that all for `Fortran_main` is +processed by the linker. This is done to issue a proper error message when +multiple definitions of `main` occur. This happens, for instance, when linking +a code that has a Fortran program unit with a C/C++ code that also defines a +`main` function. + +If the code is C/C++ based and invokes Fortran routines, either use Clang as the +linker driver (supplying `FortranRuntime` and/or `FortranDecimal` to the linker mjklemm wrote: > I think this requirement of using Clang as the linker driver if the `main` is > C changes the usability for users coming from XLF or gfortran. I can reword this. There are two options: either do C/C++ linkage and supply Fortran libraries or do Fortran linkage and supply C/C++ libraries. Either way is equally difficult at this point. > I think the fundamental question is why Fortran_main.c is inserting a "fake" > main on the Fortran side when there is not user defined main in Fortrran code. This was inherited from Classic Flang and is also done by other compilers (e.g., Intel). > Is it possible to only creating a main when users define one in Fortran code? That would also be possible. I guess the compiler could generate all of `main` in IR and emit it. That's what GFortran does. https://github.com/llvm/llvm-project/pull/75816 ___ 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"
[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)
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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,13 @@ +! RUN: %clang -x c -o %t.c-part -c %s.c-part mjklemm wrote: That does not seem to work and gives me this error when running the test. `error: unknown integrated tool '-cc1'. Valid tools include '-fc1'.` I do not necessarily need `clang` as the C compiler for the test. GCC (or whatever was used to build that instance of LLVM) would also be sufficient. I guess that problem will come back when testing `BIND(C)`. So, maybe a more generic and better solution might be needed. But I guess that may be beyond this PR. 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: 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,15 @@ +! UNSUPPORTED: system-windows + +! RUN: %clang -o %t.c-object -c %S/Inputs/main_dupes.c mjklemm wrote: When I do this, the test goes to "unsupported" for me, as it seems that %cc is not set as a substitution in lit.cfg.py. 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,13 @@ +! RUN: %clang -x c -o %t.c-part -c %s.c-part mjklemm wrote: That has worked. I haven't thought about that at all, so many thanks for the suggestion. This greatly simplifies things! 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
[flang] [clang] [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/9] 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/9] 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/9] 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)
@@ -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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +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__); mjklemm wrote: This is also gone with the move to an LL file. 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
@@ -0,0 +1,15 @@ +! UNSUPPORTED: system-windows + +! RUN: %clang -o %t.c-object -c %S/Inputs/main_dupes.c mjklemm wrote: This change is also gone now and has been removed. 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)
@@ -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
[flang] [clang] [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 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 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 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 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 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 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" "-lFortranRun
[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)
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)
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
[flang] [clang] [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/12] 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/12] 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/12] 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > I think so yes. The issue is that Windows really wants these things to be in > the object files rather than passed on the link line, which is what the patch > you're referencing changed for linking the runtimes at least. Doing that for > this as well is a little more complex though, so I suggest just passing the > `/WHOLEARCHIVE` on the link line as the runtimes were also being passed > before (you won't need to pass Fortran_main like it was before though, as the > directive to link that is now in every object file). Please have a look at the commit that I have just pushed. It uses `/WHOLEARCHIVE:Fortran_main.static.lib` (and same for the other three versions), because I do not want `/WHOLEARCHIVE` to be effective for all of the link line. I know this a duplication, but if the final fix too complex for now, this seems to be a good alternative for now. If y'all agree, then I think we can proceed with a squash merge. 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
[flang] [clang] [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: I can rephrase that comment into something like this: ``` // We are using this --whole-archive/--no-whole-archive bracket w/o // any further checks, because a user should correctly specify // -Wl,--whole-archive/-Wl,--no-whole-archive around specific // library (e.g., -Wl,--whole-archive -ldummy -Wl,--no-whole-archive). ``` Would be more acceptable? 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
https://github.com/mjklemm 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)
@@ -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)
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)
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
[flang] [clang] [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)
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
[flang] [clang] [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: Are you sure? The default is to modify the link line and only not do it if --whole-archive was found. However, there's an actual bug now, as this goes is treated wrongly: `-Wl,--whole-archive -ldummy -Wl,-dummy` Now, the check does not see the --whole-archive anymore. I need to go back to go through all the -Wl, and see which is the last one to have something to do with --whole-archive. 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
[flang] [clang] [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: Right. I'll fix this when I fixup the detecting algorithm. The getLastArg thing was a red herring :-) 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
[flang] [clang] [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); +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); 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)
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)
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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: gfortran and also ifort should fail for codes that have both a program unit and a makn() function coming from C. These compilers have a main() function that calls into the Fortran program unit and link that via an object file. Id be interested to see the link line for these compilers before we revert the PR. The behavior now should actually be consistent with these other compilers. 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: Here's the reproducer on my system: ``` [2023-12-01 18:36:31 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> gfortran -o ftn.o -c ftn.f90 && gcc -o prg.o -c prg.c && gfortran -o ./bla ftn.o prg.o /usr/bin/ld: prg.o: in function `main': prg.c:(.text+0x0): multiple definition of `main'; ftn.o:ftn.f90:(.text+0x8f): first defined here collect2: error: ld returned 1 exit status [2023-12-01 18:36:34 CET] [1] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> cat prg.c #include int main(int argc, char * argv[]) { printf("Hello from C [%s]\n", __FUNCTION__); return 0; } [2023-12-01 18:36:37 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> cat ftn.f90 program ftn print '(A)', 'Hello from Fortran' end program ftn [2023-12-01 18:36:40 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> gfortran --version GNU Fortran (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 Copyright (C) 2021 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ``` 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
[flang] [clang] [flang][Driver] Let the linker fail on multiple definitions of main() (PR #73124)
mjklemm wrote: > That example is different because you used `program` in the fortran source. > The use case here is applications which implement `main` in C then call into > fortran code Do you have a link line for ifort? It seems that your example fails with ifort, but indeed works with gfortran. The simple reason is that gfortran emits a main function into the translation unit if it has a program unit. If it does not, the main function is not generated. 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] Revert "[flang][Driver] Let the linker fail on multiple definitions of main()" (PR #74120)
mjklemm wrote: The above fails with ifort/ifx: ``` [2023-12-01 19:00:56 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> cat > ftn.f90 function pow(a, b) real :: a, b, pow pow = a ** b end function [2023-12-01 19:01:03 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> !if [2023-12-01 19:01:04 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> ifx -o ftn.o -c ftn.f90 && icc -o prg.o -c prg.c && ifx -o bla prg.o ftn.o icc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message. ld: prg.o: in function `main': prg.c:(.text+0x0): multiple definition of `main'; /net/software/x86_64/oneapi/compiler/2023.1.0/linux/compiler/lib/intel64_lin/for_main.o:for_main.c:(.text+0x0): first defined here ld: /net/software/x86_64/oneapi/compiler/2023.1.0/linux/compiler/lib/intel64_lin/for_main.o: in function `main': for_main.c:(.text+0x19): undefined reference to `MAIN__' [2023-12-01 19:01:24 CET] [1] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> cat prg.c int main(void) { // call fortran code return 0; } [2023-12-01 19:01:27 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> cat ftn.f90 function pow(a, b) real :: a, b, pow pow = a ** b end function [2023-12-01 19:01:31 CET] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> ifx -o ftn.o -c ftn.f90 && icc -o prg.o -c prg.c && ifx -o bla prg.o ftn.o icc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message. ld: prg.o: in function `main': prg.c:(.text+0x0): multiple definition of `main'; /net/software/x86_64/oneapi/compiler/2023.1.0/linux/compiler/lib/intel64_lin/for_main.o:for_main.c:(.text+0x0): first defined here ld: /net/software/x86_64/oneapi/compiler/2023.1.0/linux/compiler/lib/intel64_lin/for_main.o: in function `main': for_main.c:(.text+0x19): undefined reference to `MAIN__' [2023-12-01 19:01:34 CET] [1] iris ~/tm*/fo*/ftn_main_dupes [0:0] (main *=)> ifort -o ftn.o -c ftn.f90 && icc -o prg.o -c prg.c && ifort -o bla prg.o ftn.o icc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message. ld: prg.o: in function `main': prg.c:(.text+0x0): multiple definition of `main'; /net/software/x86_64/oneapi/compiler/2023.1.0/linux/bin/intel64/../../compiler/lib/intel64_lin/for_main.o:for_main.c:(.text+0x0): first defined here ld: /net/software/x86_64/oneapi/compiler/2023.1.0/linux/bin/intel64/../../compiler/lib/intel64_lin/for_main.o: in function `main': for_main.c:(.text+0x19): undefined reference to `MAIN__' ``` To get gfortran's behavior one could also use `-Wl,--allow-multiple-definition` which would then allow the linker to pick the C `main` function and ignore the `main` function from Fortran_main.a https://github.com/llvm/llvm-project/pull/74120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] Revert "[flang][Driver] Let the linker fail on multiple definitions of main()" (PR #74120)
mjklemm wrote: > For what it's worth, `armflang` follows gfortran's behavior. So does the "old legacy flang", which I guess is the basis for armflang. AOCC's flang shows the same behavior. > As there isn't consensus amongst fortran compilers, I'm unsure which behavior > we should follow. What do other's think? I guess, we could agree that the current behavior of flang-new ignoring the Fortran program unit is present, is not correct either. :-) Could please see if `-Wl,--allow-multiple-definition` would help to resolve the issue in the application? https://github.com/llvm/llvm-project/pull/74120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] Revert "[flang][Driver] Let the linker fail on multiple definitions of main()" (PR #74120)
mjklemm wrote: BTW, flang legacy has this: ` -fno-fortran-main Don't link in Fortran main` Adding that command line option might be the right choice. If everyone agrees, I can see if I can get this added. https://github.com/llvm/llvm-project/pull/74120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm created https://github.com/llvm/llvm-project/pull/74139 This is related to PR #74120 and (merged) PR #73124. This PR adds the `-fno-fortran-main` command line option to remove `Fortran_main.a` from the link and to allow for linking Fortran code w/o program unit with C/C++ translation units that provide the `main()` entrypoint. >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/2] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d68..aa26344f67b3132 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/2] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e94a..2899f07cb7490ca 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] Revert "[flang][Driver] Let the linker fail on multiple definitions of main()" (PR #74120)
mjklemm wrote: I have opened PR #74139 to provide an alternate solution to this. Please have a look there. @tblah Would you mind trying this new option with one application code to see if that will help? https://github.com/llvm/llvm-project/pull/74120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
mjklemm wrote: This WIP for now, as I still have to make the changes for MSVC. But it's good enough to discuss if this is a viable solution. https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang][driver][RFC] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm edited https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver][RFC] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/4] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d68..aa26344f67b3132 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/4] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e94a..2899f07cb7490ca 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/4] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490ca..f0e3df2a63ae989 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm edited https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/5] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d..aa26344f67b31 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/5] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e9..2899f07cb7490 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/5] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490..f0e3df2a63ae9 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Dr
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
mjklemm wrote: > I think this solution is fine, at least in the short term. > > I had a think after reviewing the initial patch and looking at the failure > that @tblah showed in #73124; my thoughts are that the “correct” way of doing > this would be instead of linking Fortran_main all the time, we could insert a > linker directive in the object file containing the program statement. That > way we would only link Fortran_main when there is actually a program > statement in the Fortran. However that’s more work and would need a bit more > thought anyway so we at least need something like this or the revert in the > interim. That sounds good! For now, the code should effectively avoid linking Fortran_main on Linux and Windows if -fno-fortran-main is present. https://github.com/llvm/llvm-project/pull/74139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/5] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d..aa26344f67b31 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/5] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e9..2899f07cb7490 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/5] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490..f0e3df2a63ae9 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Dr
[clang] [flang] [flang][driver] Add -fno-fortran-main (link time) option to remove Fortran_main from link line (PR #74139)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/74139 >From 2e41335a7de3d2efa88eacee659172a3b9525e45 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:41:44 +0100 Subject: [PATCH 1/6] Add -fno-fortran-main driver option --- clang/include/clang/Driver/Options.td | 4 1 file changed, 4 insertions(+) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19d04e82aed4d..aa26344f67b31 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6345,6 +6345,10 @@ def J : JoinedOrSeparate<["-"], "J">, Group, Alias; +def no_fortran_main : Flag<["-"], "fno-fortran-main">, + Visibility<[FlangOption]>, Group, + HelpText<"Don't link in Fortran main">; + //===--===// // FC1 Options //===--===// >From 44b684ae43a6da37bb56c5b699628c6807257ad9 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 21:42:09 +0100 Subject: [PATCH 2/6] Skip linking Fortran_main.a if -fno-fortran-main is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 46 -- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0ae8e2dce32e9..2899f07cb7490 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1116,33 +1116,37 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC, return true; } +// TODO: add -fno-fortran-main option and check it in this function. void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // These are handled earlier on Windows by telling the frontend driver to add // the correct libraries to link against as dependents in the object file. if (!TC.getTriple().isKnownWindowsMSVCEnvironment()) { -// The --whole-archive option needs to be part of the link line to -// make sure that the main() function from Fortran_main.a is pulled -// in by the linker. Determine if --whole-archive is active when -// flang will try to link Fortran_main.a. If it is, don't add the -// --whole-archive flag to the link line. If it's not, add a proper -// --whole-archive/--no-whole-archive bracket to the link line. -bool WholeArchiveActive = false; -for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) - if (Arg) -for (StringRef ArgValue : Arg->getValues()) { - if (ArgValue == "--whole-archive") -WholeArchiveActive = true; - if (ArgValue == "--no-whole-archive") -WholeArchiveActive = false; -} - -if (!WholeArchiveActive) - CmdArgs.push_back("--whole-archive"); -CmdArgs.push_back("-lFortran_main"); -if (!WholeArchiveActive) - CmdArgs.push_back("--no-whole-archive"); +// if -fno-fortran-main has been passed, skip linking Fortran_main.a +bool DontLinkFortranMain = Args.getLastArg(options::OPT_no_fortran_main) != nullptr; +if (!DontLinkFortranMain) { + // The --whole-archive option needs to be part of the link line to + // make sure that the main() function from Fortran_main.a is pulled + // in by the linker. Determine if --whole-archive is active when + // flang will try to link Fortran_main.a. If it is, don't add the + // --whole-archive flag to the link line. If it's not, add a proper + // --whole-archive/--no-whole-archive bracket to the link line. + bool WholeArchiveActive = false; + for (auto *Arg : Args.filtered(options::OPT_Wl_COMMA)) +if (Arg) + for (StringRef ArgValue : Arg->getValues()) { +if (ArgValue == "--whole-archive") + WholeArchiveActive = true; +if (ArgValue == "--no-whole-archive") + WholeArchiveActive = false; + } + if (!WholeArchiveActive) +CmdArgs.push_back("--whole-archive"); + CmdArgs.push_back("-lFortran_main"); + if (!WholeArchiveActive) +CmdArgs.push_back("--no-whole-archive"); +} // Perform regular linkage of the remaining runtime libraries. CmdArgs.push_back("-lFortranRuntime"); CmdArgs.push_back("-lFortranDecimal"); >From e1a472fa914ea9405be6589e89fbe8201448600f Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Fri, 1 Dec 2023 15:10:29 -0600 Subject: [PATCH 3/6] Cleanup and simplify the code a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 2899f07cb7490..f0e3df2a63ae9 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Dr
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,40 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver +When used as a linker, Flang's frontend driver assembles the command line for an +external linker command (e.g., LLVM's `lld`) and invokes it to create the final +executable by linking static and shared libraries together with all the +translation units supplied as object files. + +By default, the Flang linker driver adds several libraries to the linker +invocation to make sure that all entrypoints for program start +(Fortran's program unit) and runtime routines can be resolved by the linker. +The libraries are: + +* `Fortran_main`: Provides the main entry point `main` that then invokes + `_QQmain` with the Fortran program unit. This library has a dependency to + the `FortranRuntime` library. +* `FortranRuntime`: Provides most of the Flang runtime library. +* `FortranDecimal`: Provides operations for decimal numbers. + +The default is that, when using Flang as the linker, one of the Fortran +translation units provides the program unit and therefore it is assumed that +Fortran is the main code part (calling into C/C++ routines via `BIND +(C)` interfaces). When composing the linker commandline, Flang uses +`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`, +Darwin: *not implemented yet*) to make sure that all for `Fortran_main` is +processed by the linker. This is done to issue a proper error message when +multiple definitions of `main` occur. This happens, for instance, when linking +a code that has a Fortran program unit with a C/C++ code that also defines a +`main` function. + +If the code is C/C++ based and invokes Fortran routines, either use Clang as the +linker driver (supplying `FortranRuntime` and/or `FortranDecimal` to the linker mjklemm wrote: > It could be updated, but that should probably be raised on Discourse or in > the community call. Having said, documenting this is long overdue and I am > super grateful that Michael is doing this for us 🙏🏻 I fully agree. The good thing is: it's not defined in any (ISO) standard, so we can do whatever we like and what we think is useful behavior in the end. I'd like to note that the introduction of `-fno-fortran-main` can still be useful with a behavior that is similar to GFortran (emitting the actual `main` code in the translation unit with a program statement), but would then turn `-fno-fortran-main` flag into a more corner-case, expert type flag. I'd suggest to get current behavior cleanly merged for the 18.0 branch and then re-think this somewhat more for the 19.0 release and the future. At least now, we will have documented and well-defined behavior. If it's the right one, we can debate. :-) https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,40 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver mjklemm wrote: Good suggestion! https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [Flang] remove whole-archive option for AIX linker (PR #76039)
https://github.com/mjklemm approved this pull request. LGTM, but please add to the TODO comment in line 1177 that for AIX a better solution needs to be found. https://github.com/llvm/llvm-project/pull/76039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/75816 >From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Sat, 16 Dec 2023 20:15:17 +0100 Subject: [PATCH 1/5] Remove -lFortran_main from the link line when -shared is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ 1 file changed, 22 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 45901ee7157f77..5d525d3794ad1d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +static bool isSharedLinkage(const ArgList &Args) { + bool FoundSharedFlag = false; + for (auto *Arg : Args.filtered(options::OPT_shared)) { +if (Arg) { + FoundSharedFlag = true; +} + } + + return FoundSharedFlag; +} + /// Add Fortran runtime libs for MSVC static void addFortranRuntimeLibsMSVC(const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { @@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, // Add FortranMain runtime lib static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { + // 0. Shared-library linkage + // If we are attempting to link a shared library, we should not add + // -lFortran_main.a to the link line, as the `main` symbol is not + // required for a shared library and should also be provided by one + // of the translation units of the code that this shared library + // will be linked against eventually. + if (isSharedLinkage(Args)) { +printf("MK: --> shared linkage, do not add -lFortranMain\n"); +return; + } + // 1. MSVC if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { addFortranRuntimeLibsMSVC(Args, CmdArgs); >From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Mon, 18 Dec 2023 11:27:59 +0100 Subject: [PATCH 2/5] Update dynamic_linker.f90 test and clean up a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ flang/test/Driver/dynamic-linker.f90 | 8 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 5d525d3794ad1d..05ebd42829c95d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +/// Determine if driver is invoked to create a shared object library (-static) static bool isSharedLinkage(const ArgList &Args) { - bool FoundSharedFlag = false; - for (auto *Arg : Args.filtered(options::OPT_shared)) { -if (Arg) { - FoundSharedFlag = true; -} - } + return Args.hasArg(options::OPT_shared); +} - return FoundSharedFlag; +/// Determine if driver is invoked to create a static object library (-shared) +static bool isStaticLinkage(const ArgList &Args) { + return Args.hasArg(options::OPT_static); } /// Add Fortran runtime libs for MSVC @@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // 0. Shared-library linkage - // If we are attempting to link a shared library, we should not add + // If we are attempting to link a library, we should not add // -lFortran_main.a to the link line, as the `main` symbol is not - // required for a shared library and should also be provided by one - // of the translation units of the code that this shared library + // required for a library and should also be provided by one of + // the translation units of the code that this shared library // will be linked against eventually. - if (isSharedLinkage(Args)) { -printf("MK: --> shared linkage, do not add -lFortranMain\n"); + if (isSharedLinkage(Args) || isStaticLinkage(Args)) { return; } diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90 index df119c22a2ea51..af07e2483f93fa 100644 --- a/flang/test/Driver/dynamic-linker.f90 +++ b/flang/test/Driver/dynamic-linker.f90 @@ -3,18 +3,22 @@ ! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s +! RUN: --check-prefixes=GNU-LINKER-OPTIONS \ +! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s ! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s +! RUN:
[clang] [flang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,40 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver mjklemm wrote: I have tried to capture this now. https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,40 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver +When used as a linker, Flang's frontend driver assembles the command line for an +external linker command (e.g., LLVM's `lld`) and invokes it to create the final +executable by linking static and shared libraries together with all the +translation units supplied as object files. + +By default, the Flang linker driver adds several libraries to the linker +invocation to make sure that all entrypoints for program start +(Fortran's program unit) and runtime routines can be resolved by the linker. +The libraries are: + +* `Fortran_main`: Provides the main entry point `main` that then invokes + `_QQmain` with the Fortran program unit. This library has a dependency to + the `FortranRuntime` library. +* `FortranRuntime`: Provides most of the Flang runtime library. +* `FortranDecimal`: Provides operations for decimal numbers. + +The default is that, when using Flang as the linker, one of the Fortran +translation units provides the program unit and therefore it is assumed that +Fortran is the main code part (calling into C/C++ routines via `BIND +(C)` interfaces). When composing the linker commandline, Flang uses +`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`, +Darwin: *not implemented yet*) to make sure that all for `Fortran_main` is +processed by the linker. This is done to issue a proper error message when +multiple definitions of `main` occur. This happens, for instance, when linking +a code that has a Fortran program unit with a C/C++ code that also defines a +`main` function. + +If the code is C/C++ based and invokes Fortran routines, either use Clang as the +linker driver (supplying `FortranRuntime` and/or `FortranDecimal` to the linker mjklemm wrote: I have tried to clarify a bit what the two choices are when using Clang vs Flang to link a program. Is this better now? https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/75816 >From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Sat, 16 Dec 2023 20:15:17 +0100 Subject: [PATCH 1/6] Remove -lFortran_main from the link line when -shared is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ 1 file changed, 22 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 45901ee7157f77..5d525d3794ad1d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +static bool isSharedLinkage(const ArgList &Args) { + bool FoundSharedFlag = false; + for (auto *Arg : Args.filtered(options::OPT_shared)) { +if (Arg) { + FoundSharedFlag = true; +} + } + + return FoundSharedFlag; +} + /// Add Fortran runtime libs for MSVC static void addFortranRuntimeLibsMSVC(const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { @@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, // Add FortranMain runtime lib static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { + // 0. Shared-library linkage + // If we are attempting to link a shared library, we should not add + // -lFortran_main.a to the link line, as the `main` symbol is not + // required for a shared library and should also be provided by one + // of the translation units of the code that this shared library + // will be linked against eventually. + if (isSharedLinkage(Args)) { +printf("MK: --> shared linkage, do not add -lFortranMain\n"); +return; + } + // 1. MSVC if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { addFortranRuntimeLibsMSVC(Args, CmdArgs); >From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Mon, 18 Dec 2023 11:27:59 +0100 Subject: [PATCH 2/6] Update dynamic_linker.f90 test and clean up a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ flang/test/Driver/dynamic-linker.f90 | 8 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 5d525d3794ad1d..05ebd42829c95d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +/// Determine if driver is invoked to create a shared object library (-static) static bool isSharedLinkage(const ArgList &Args) { - bool FoundSharedFlag = false; - for (auto *Arg : Args.filtered(options::OPT_shared)) { -if (Arg) { - FoundSharedFlag = true; -} - } + return Args.hasArg(options::OPT_shared); +} - return FoundSharedFlag; +/// Determine if driver is invoked to create a static object library (-shared) +static bool isStaticLinkage(const ArgList &Args) { + return Args.hasArg(options::OPT_static); } /// Add Fortran runtime libs for MSVC @@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // 0. Shared-library linkage - // If we are attempting to link a shared library, we should not add + // If we are attempting to link a library, we should not add // -lFortran_main.a to the link line, as the `main` symbol is not - // required for a shared library and should also be provided by one - // of the translation units of the code that this shared library + // required for a library and should also be provided by one of + // the translation units of the code that this shared library // will be linked against eventually. - if (isSharedLinkage(Args)) { -printf("MK: --> shared linkage, do not add -lFortranMain\n"); + if (isSharedLinkage(Args) || isStaticLinkage(Args)) { return; } diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90 index df119c22a2ea51..af07e2483f93fa 100644 --- a/flang/test/Driver/dynamic-linker.f90 +++ b/flang/test/Driver/dynamic-linker.f90 @@ -3,18 +3,22 @@ ! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s +! RUN: --check-prefixes=GNU-LINKER-OPTIONS \ +! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s ! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s +! RUN:
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,62 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver +When used as a linker, Flang's frontend driver assembles the command line for an +external linker command (e.g., LLVM's `lld`) and invokes it to create the final +executable by linking static and shared libraries together with all the +translation units supplied as object files. + +By default, the Flang linker driver adds several libraries to the linker +invocation to make sure that all entrypoints for program start +(Fortran's program unit) and runtime routines can be resolved by the linker. + +An abridged example (only showing the Fortran specific linker flags, omission +indicated by `[...]`) for such a linker invocation on a Linux system would look +like this: + +``` +$ flang -v -o example example.o +"/usr/bin/ld" [...] example.o [...] "--whole-archive" "-lFortran_main" +"--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal" [...] +``` + +The automatically added libraries are: + +* `Fortran_main`: Provides the main entry point `main` that then invokes + `_QQmain` with the Fortran program unit. This library has a dependency to + the `FortranRuntime` library. +* `FortranRuntime`: Provides most of the Flang runtime library. +* `FortranDecimal`: Provides operations for decimal numbers. + +The default is that, when using Flang as the linker, one of the Fortran +translation units provides the program unit and therefore it is assumed that +Fortran is the main code part (calling into C/C++ routines via `BIND (C)` +interfaces). When composing the linker commandline, Flang uses +`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`, +Darwin & AIX: *not implemented yet*) to make sure that all for `Fortran_main` +is processed by the linker. This is done to issue a proper error message when +multiple definitions of `main` occur. This happens, for instance, when linking +a code that has a Fortran program unit with a C/C++ code that also defines a +`main` function. A user may be required to explicitly provide the C++ runtime +libraries at link time (e.g., via `-lstdc++` for STL) + +If the code is C/C++ based and invokes Fortran routines, one can either use Clang +for Flang as the linker driver. If Clang is used, it will automatically all +required runtime libraries needed by C++ (e.g., for STL) to the linker invocation. mjklemm wrote: I treated clang and clang++ the same here, as they seem to have reasonable logic to figure out if C or C++ is compiled and linked, https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
@@ -163,6 +163,62 @@ forward compiler options to the frontend driver, `flang-new -fc1`. You can read more on the design of `clangDriver` in Clang's [Driver Design & Internals](https://clang.llvm.org/docs/DriverInternals.html). +## Linker Driver +When used as a linker, Flang's frontend driver assembles the command line for an +external linker command (e.g., LLVM's `lld`) and invokes it to create the final +executable by linking static and shared libraries together with all the +translation units supplied as object files. + +By default, the Flang linker driver adds several libraries to the linker +invocation to make sure that all entrypoints for program start +(Fortran's program unit) and runtime routines can be resolved by the linker. + +An abridged example (only showing the Fortran specific linker flags, omission +indicated by `[...]`) for such a linker invocation on a Linux system would look +like this: + +``` +$ flang -v -o example example.o +"/usr/bin/ld" [...] example.o [...] "--whole-archive" "-lFortran_main" +"--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal" [...] +``` + +The automatically added libraries are: + +* `Fortran_main`: Provides the main entry point `main` that then invokes + `_QQmain` with the Fortran program unit. This library has a dependency to + the `FortranRuntime` library. +* `FortranRuntime`: Provides most of the Flang runtime library. +* `FortranDecimal`: Provides operations for decimal numbers. + +The default is that, when using Flang as the linker, one of the Fortran +translation units provides the program unit and therefore it is assumed that +Fortran is the main code part (calling into C/C++ routines via `BIND (C)` +interfaces). When composing the linker commandline, Flang uses +`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`, +Darwin & AIX: *not implemented yet*) to make sure that all for `Fortran_main` +is processed by the linker. This is done to issue a proper error message when +multiple definitions of `main` occur. This happens, for instance, when linking +a code that has a Fortran program unit with a C/C++ code that also defines a +`main` function. A user may be required to explicitly provide the C++ runtime +libraries at link time (e.g., via `-lstdc++` for STL) + +If the code is C/C++ based and invokes Fortran routines, one can either use Clang +for Flang as the linker driver. If Clang is used, it will automatically all +required runtime libraries needed by C++ (e.g., for STL) to the linker invocation. +In this case, one has to explicitly provide the Fortran runtime libraries +`FortranRuntime` and/or `FortranDecimal`. An alternative is to use Flang to link +and use the `-fno-fortran-main` flag. This flag removes +`Fortran_main` from the linker stage and hence requires one of the C/C++ +translation units to provide a definition of the `main` function. In this case, +it may be required to explicitly supply C++ runtime libraries as mentioned above. + +When creating shared or static libraries, `Fortran_main` is automatically mjklemm wrote: Applied! Thanks for the rewording! https://github.com/llvm/llvm-project/pull/75816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [flang][driver] Remove Fortain_main static library from linking stages (PR #75816)
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/75816 >From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Sat, 16 Dec 2023 20:15:17 +0100 Subject: [PATCH 1/7] Remove -lFortran_main from the link line when -shared is present --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ 1 file changed, 22 insertions(+) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 45901ee7157f77..5d525d3794ad1d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +static bool isSharedLinkage(const ArgList &Args) { + bool FoundSharedFlag = false; + for (auto *Arg : Args.filtered(options::OPT_shared)) { +if (Arg) { + FoundSharedFlag = true; +} + } + + return FoundSharedFlag; +} + /// Add Fortran runtime libs for MSVC static void addFortranRuntimeLibsMSVC(const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { @@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, // Add FortranMain runtime lib static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { + // 0. Shared-library linkage + // If we are attempting to link a shared library, we should not add + // -lFortran_main.a to the link line, as the `main` symbol is not + // required for a shared library and should also be provided by one + // of the translation units of the code that this shared library + // will be linked against eventually. + if (isSharedLinkage(Args)) { +printf("MK: --> shared linkage, do not add -lFortranMain\n"); +return; + } + // 1. MSVC if (TC.getTriple().isKnownWindowsMSVCEnvironment()) { addFortranRuntimeLibsMSVC(Args, CmdArgs); >From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Mon, 18 Dec 2023 11:27:59 +0100 Subject: [PATCH 2/7] Update dynamic_linker.f90 test and clean up a bit --- clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++ flang/test/Driver/dynamic-linker.f90 | 8 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 5d525d3794ad1d..05ebd42829c95d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) { return WholeArchiveActive; } +/// Determine if driver is invoked to create a shared object library (-static) static bool isSharedLinkage(const ArgList &Args) { - bool FoundSharedFlag = false; - for (auto *Arg : Args.filtered(options::OPT_shared)) { -if (Arg) { - FoundSharedFlag = true; -} - } + return Args.hasArg(options::OPT_shared); +} - return FoundSharedFlag; +/// Determine if driver is invoked to create a static object library (-shared) +static bool isStaticLinkage(const ArgList &Args) { + return Args.hasArg(options::OPT_static); } /// Add Fortran runtime libs for MSVC @@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList &Args, static void addFortranMain(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { // 0. Shared-library linkage - // If we are attempting to link a shared library, we should not add + // If we are attempting to link a library, we should not add // -lFortran_main.a to the link line, as the `main` symbol is not - // required for a shared library and should also be provided by one - // of the translation units of the code that this shared library + // required for a library and should also be provided by one of + // the translation units of the code that this shared library // will be linked against eventually. - if (isSharedLinkage(Args)) { -printf("MK: --> shared linkage, do not add -lFortranMain\n"); + if (isSharedLinkage(Args) || isStaticLinkage(Args)) { return; } diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90 index df119c22a2ea51..af07e2483f93fa 100644 --- a/flang/test/Driver/dynamic-linker.f90 +++ b/flang/test/Driver/dynamic-linker.f90 @@ -3,18 +3,22 @@ ! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s +! RUN: --check-prefixes=GNU-LINKER-OPTIONS \ +! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s ! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \ ! RUN: -static %s 2>&1 | FileCheck \ -! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s +! RUN: