[PATCH] D126291: [flang][Driver] Update link job on windows

2022-08-05 Thread Amy Kwan via Phabricator via cfe-commits
amyk added a comment. Hi, I have come across a failure involving `flang/test/Driver/linker-flags.f90` that occurs when the default linker is lld. I have opened an external issue for it here: https://github.com/llvm/llvm-project/issues/56955 Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-20 Thread Diana Picus via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG26041e17006c: Update link job for flang on windows (authored by rovka). Changed prior to commit: https://reviews.llvm.org/D126291?vs=436722&id=438

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-17 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:773 + // TODO: Make this work unconditionally once Flang is mature enough. + if (!Args.hasArg(options::OPT_flang_experimental_exec)) +return; rovka wrote: > mstorsjo wrot

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-17 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I'm going to go ahead and commit this on Monday if nobody raises any objections until then. Thanks again to everyone for all the help & feedback! Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:773 + // TODO: Make this work unconditionally once F

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-16 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. Looks good to me in general, one general question though. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:773 + // TODO: Make this work unconditionally once Flang is mature enough. + if (!Args.hasArg(options::OPT_flang_experimental_exec)) +

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-16 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, many thanks for this non trivial effort! :) I've left a few nits, feel free to ignore! @mstorsjo , are you also OK with this change? [nit] "This is exactly what we do for Linux/D

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-15 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" awarzynski wrote: > rovka wrote: > > awarzynski wrote: > > > rovka wrote: > > > > mmuetzel wrote: > > > > > Li

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" rovka wrote: > awarzynski wrote: > > rovka wrote: > > > mmuetzel wrote: > > > > Lines 50-51 seem to be du

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436722. rovka added a comment. - Use --implicit-not-check <3 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolChains/CommonArgs.h c

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" awarzynski wrote: > rovka wrote: > > mmuetzel wrote: > > > Lines 50-51 seem to be duplicates of lines 44-45. I

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" rovka wrote: > mmuetzel wrote: > > Lines 50-51 seem to be duplicates of lines 44-45. Is this intentional?

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" mmuetzel wrote: > Lines 50-51 seem to be duplicates of lines 44-45. Is this intentional? Yes, I don't want tho

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" Lines 50-51 seem to be duplicates of lines 44-45. Is this intentional? CHANGES SINCE LAST ACTION https:

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436687. rovka added a comment. Clarified test checks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolChains/CommonArgs.h clang/li

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added inline comments. Comment at: flang/test/Driver/linker-flags.f90:28 + +! GNU-WITHOUTLM-LABEL: "{{.*}}ld" +! GNU-WITHOUTLM-SAME: "[[object_file]]" I think that GNU in this case might be a bit misleading. These linker invocations are defined in a

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436340. rovka added a comment. - Stop using --ld-path, since it seems to be ignored on Windows. Instead, just match any path that ends in ld (should work with ld, lld, gold). This isn't very robust, but I don't have any better ideas (other than actually suppor

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I'm guessing the Windows precommit is failing because --ld-path is ignored on Windows, even if we use a Linux target? I have a fix that works on my Windows machine, coming right up. Comment at: flang/test/Driver/linker-flags.f90:28 +! GNU-SAME: -lFortra

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. 🤔 linker-flags.f90 is failing on Windows in the pre-commit CI. Not sure why - seems fine on Debian. Comment at: flang/test/Driver/linker-flags.f90:28 +! GNU-SAME: -lFortranDecimal +! WITHLM-SAME: -lm + Does `-SAME` makese sense here

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. In D126291#3577133 , @rovka wrote: > I had the same idea about switching the tests to using target triples instead > of having separate files for it, but initially I had some issues getting that > to work properly. Could you

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3577159 , @mstorsjo wrote: > In D126291#3577133 , @rovka wrote: > >> I had the same idea about switching the tests to using target triples >> instead of having separate files for

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3572796 , @mmuetzel wrote: > In D126291#3572777 , @rovka wrote: > >> Moved the check for `-flang-experimental-exec` into >> addFortranRuntimeLibraryPath, so it affects all the to

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3577133 , @rovka wrote: > I had the same idea about switching the tests to using target triples instead > of having separate files for it, but initially I had some issues getting that > to work properly. When specify

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436301. rovka added a comment. Missed a spot (removing the namespace in MinGW.cpp) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolC

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436300. rovka added a comment. Herald added a subscriber: ormris. - Switch to the new style of testing (using triples rather than separate files) - Fix oversight in Darwin toolchain file I hope I've addressed all the comments and nothing slipped past during reb

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I had the same idea about switching the tests to using target triples instead of having separate files for it, but initially I had some issues getting that to work properly. When specifying a triple, we need to provide an architecture. Leaving the triple as `unkown-linux-

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-13 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added inline comments. Comment at: flang/test/Driver/linker-flags-windows.f90:6 +! NOTE: The additional linker flags tested here are currently specified in +! clang/lib/Driver/Toolchains/MSVC.cpp. +! REQUIRES: windows-msvc mmuetzel wrote: > Since this te

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-11 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. Possible alternative version of `linker-flags-windows.f90` that also tests MinGW and takes the feedback by @mstorsjo into account: ! Verify that the Fortran runtime libraries are present in the linker ! invocation. These libraries are added on top of other standard

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:645 if (getToolChain().getDriver().IsFlangMode() && Args.hasArg(options::OPT_flang_experimental_exec)) { addFortranRuntimeLibraryPath(getToolChain(), Args, CmdArgs);

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3573937 , @mmuetzel wrote: > Ah ok. So instead of having `! REQUIRES:` lines, those tests should add > `-target x86_64-windows-msvc` to the flang invocation? Exactly > On platforms that didn't build the necessary li

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3573751 , @mstorsjo wrote: > Maybe, but keep in mind that those kinds of tests should be the exception, > not the rule. > > As clang (and flang too, I would presume) generally can be cross compiling, > one shouldn't

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3573742 , @mmuetzel wrote: > In D126291#3573607 , @mstorsjo > wrote: > >> FWIW, something very similar was added just a couple days ago in an lldb >> specific lit.cfg.py: htt

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3573607 , @mstorsjo wrote: > FWIW, something very similar was added just a couple days ago in an lldb > specific lit.cfg.py: https://reviews.llvm.org/D127048#change-KJ7QgKPHtN1S Thank you for pointing this out. Those

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3573137 , @mmuetzel wrote: > Not sure if this is the right place to add this. But maybe something like > this could be used to add 'windows-msvc' and 'windows-gnu' features that > could be used to run tests condition

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. Not sure if this is the right place to add this. But maybe something like this could be used to add 'windows-msvc' and 'windows-gnu' features that could be used to run tests conditionally on Windows with MSVC or MinGW toolchains: diff --git a/llvm/utils/lit/lit/llvm/

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. In D126291#3572777 , @rovka wrote: > Moved the check for `-flang-experimental-exec` into > addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski > does this look like a good idea? OK with me! 👍🏻 CHANG

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added inline comments. Comment at: flang/test/Driver/linker-flags-windows.f90:7 +! clang/lib/Driver/Toolchains/MSVC.cpp. +! REQUIRES: windows-msvc + It looks like this test is skipped on windows (MSVC): https://buildkite.com/llvm-project/premerge-checks/

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added inline comments. Comment at: clang/lib/Driver/ToolChains/MinGW.cpp:224 +tools::addFortranRuntimeLibs(TC, CmdArgs); + } + mmuetzel wrote: > MinGW behaves similarly to GNU in many respects. The GNU toolchain file adds > `CmdArgs.push_back("-lm"

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3572777 , @rovka wrote: > Moved the check for `-flang-experimental-exec` into > addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski > does this look like a good idea? If moving that check to

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 435842. rovka added a comment. Moved the check for `-flang-experimental-exec` into addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski does this look like a good idea? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-10 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:133 + if (C.getDriver().IsFlangMode()) { +tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs); mmuetzel wrote: > The GNU toolchain has this conditional on > `Args.hasArg(opti

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-09 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. Should this be conditional on the command line flag `-flang-experimental-exec` for the time being (like for the GNU toolchain)? See D122008 Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:133 + if (C.getDriver().

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-08 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3566692 , @rovka wrote: > @mmuetzel are you going to upload it to Phab? I gave it a try on my machine > and I can't compile the runtime with it, but I'll try to fix it on my end and > we can continue the discussion i

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-08 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 435150. rovka edited the summary of this revision. rovka added a comment. - Fixed test - Unconditionally added the subsystem arg - Incorporated the MinGW toolchain changes (Thanks again @mmuetzel, I'm adding you as co-author) @awarzynski I agree that the other

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-07 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. In D126291#3562542 , @mstorsjo wrote: > In D126291#3562471 , @mmuetzel > wrote: > >> Found why I thought building on Windows wasn't supported: >> https://github.com/llvm/llvm-project

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-06 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3562471 , @mmuetzel wrote: > Found why I thought building on Windows wasn't supported: > https://github.com/llvm/llvm-project/blob/main/flang/README.md?plain=1#L153 > >> The code does not compile with Windows and a com

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-06 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3561854 , @Meinersbur wrote: > In D126291#3555639 , @awarzynski > wrote: > >> In D126291#391 , @mmuetzel >> wrote: >> >>> ISTR,

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-06 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. I looked into Google Benchmark because it also has its main in a .lib library and how it handles it. Turns out it is using cmake which by default always adds `/subsystem:console` unless setting WIN32_EXECUTABLE

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3556725 , @mstorsjo wrote: > This looks mostly reasonable, but I'd recommend using a windows critical > section instead of a mutex - a critical section doesn't invoke the kernel > when there's no contention of the lo

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3556324 , @mmuetzel wrote: > With this additional change, I no longer need the `-lc++` flag: > > From 965343d8b05bf3cf7a9a3873ea4d2ddcc00a3703 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Markus=20M=C3=BCtzel?= >

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. In D126291#3556324 , @mmuetzel wrote: > With this additional change, I no longer need the `-lc++` flag: Great find, thanks for digging! > Someone should check if that change is sane. But it seems to work for me > (with the "

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. With this additional change, I no longer need the `-lc++` flag: From 62920169c43de27569ecae9ba0ec0fe4ec01633a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=BCtzel?= Date: Fri, 3 Jun 2022 19:17:34 +0200 Subject: [PATCH] lock without on Windows ---

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3556096 , @mmuetzel wrote: > The error message I get without `-lc++`: > > ld.lld: error: undefined symbol: std::__1::mutex::lock() > >>> referenced by > libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::I

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. The error message I get without `-lc++`: ld.lld: error: undefined symbol: std::__1::mutex::lock() >>> referenced by libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Directi

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. In D126291#3555966 , @mmuetzel wrote: > In case you don't receive notifications on edits. (Please forgive the noise > if you do.) That's not a problem at all! > After applying this additional patch (not the one in the previo

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:140 +// defined in flang's runtime libraries. +if (TC.getTriple().isKnownWindowsMSVCEnvironment()) + CmdArgs.push_back("/subsystem:console"); Is the MSVC toolchain used

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In case you don't receive notifications on edits. (Please forgive the noise if you do.) After applying this additional patch: From d74a276679778b943b1e2e50f5dd45f65c530252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=BCtzel?= Date: Fri, 3 Jun 2022 16:36

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. I made this additional change: From 26cee469225e80ac9bae22ebb6e60d47373fc19d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=BCtzel?= Date: Fri, 3 Jun 2022 16:23:47 +0200 Subject: [PATCH] MinGW --- clang/lib/Driver/ToolChains/MinGW.cpp | 7 +++

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#3555848 , @awarzynski wrote: > Nice! > > In D126291#3555835 , @mmuetzel > wrote: > >> Is this still a configuration error? > > No :) Clearly the following `if` block from `to

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. Nice! In D126291#3555835 , @mmuetzel wrote: > Is this still a configuration error? No :) Clearly the following `if` block from `tools::addFortranRuntimeLibs` is not entered: if (TC.getTriple().isKnownWindowsMSVCEnvironmen

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. Thank you for the suggestions. In the meantime, I added `-DLLVM_ENABLE_PROJECTS="clang;compiler-rt;mlir;flang;llvm" -DCOMPILER_RT_USE_BUILTINS_LIBRARY=ON` to the compiler flags which seems to have advanced a bit further. Now, I get the following when trying to compile

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D126291#3555805 , @awarzynski wrote: > Like @rovka pointed out, skipping `CLANG_DEFAULT_RTLIB` should solve your > issue with missing libs Skipping that woulnd't help/work in the clang64 environment in msys2; there's no li

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. Thanks for checking @mmuetzel ! In D126291#3555780 , @mmuetzel wrote: > I ended up using these switches: > > cmake \ > -Sllvm \ > -Bbuild \ > -GNinja \ > -DCMAKE_INSTALL_PREFIX=pkg \ > -DCMAKE_C_COMPILER=

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. @mmuetzel Thanks for looking into this! I think since you're passing `-DCLANG_DEFAULT_RTLIB=compiler-rt`, you might indeed need to build compiler-rt (or at least the builtins part of it). FYI, I'll be out of office until Wednesday, but I'll definitely check all the comme

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. I checked out the LLVM repository from https://github.com/llvm/llvm-project.git and applied your patch with `patch -Np0 -i D126291.433988.patch`. After some failing attempts, I finally found a configuration for which building succeeded. I struggled with duplicate symbo

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. In D126291#391 , @mmuetzel wrote: > ISTR, I somewhere read that Windows isn't a supported host currently. Is this > no longer the case?) Windows is supported: https://lab.llvm.org/buildbot/#/builders/172 :) > If you coul

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. In D126291#356 , @rovka wrote: > - Update for MinGW. > - Add `/subsystem:console` to help `link.exe` understand what's going on. > > Thanks for all the comments. I don't have a MinGW environment readily > available for testi

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-03 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 433988. rovka added a comment. - Update for MinGW. - Add `/subsystem:console` to help `link.exe` understand what's going on. Thanks for all the comments. I don't have a MinGW environment readily available for testing. @mmuetzel Could you test this? Alternative

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-02 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. In D126291#3552573 , @rovka wrote://italic text// > I don't really know why `link.exe` doesn't work. According to the docs >

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:753 + llvm::opt::ArgStringList &CmdArgs) { + if (TC.getTriple().isOSWindows()) { +CmdArgs.push_back("Fortran_main.lib"); mmuetzel wrote: > Ar

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3550563 , @Meinersbur wrote: > While the test passes now, I still get `LINK : fatal error LNK1561: entry > point must be defined` when trying to actually link. Isn't it expected to not > work yet? > > Linking with `lld

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-01 Thread Markus Mützel via Phabricator via cfe-commits
mmuetzel added a comment. Just wondering if those changes are correct for MinGW. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:753 + llvm::opt::ArgStringList &CmdArgs) { + if (TC.getTriple().isOSWindows()) { +CmdArgs.push_back("For

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-01 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. While the test passes now, I still get `LINK : fatal error LNK1561: entry point must be defined` when trying to actually link. Isn't it expected to not work yet? Linking with `lld-link.exe` does work. objdump says there is a `_QQmain` symbol, why does lld consider t

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-01 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 433318. rovka added a comment. Updated comment in `linker-flags.f90` test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolChains/Com

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-06-01 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3547142 , @awarzynski wrote: > In D126291#3547023 , @rovka wrote: > >> With this patch in place, we still need to add `-Xlinker -subsystem:console' >> in order to compile a simp

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-31 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment. Thanks for adding this, Diana! :) Overall, makes sense to me. I have a couple of questions/points. In D126291#3547023 , @rovka wrote: > With this patch in place, we still need to add `-Xlinker -subsystem:console' > in order

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-31 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/flang-linker-flags-windows.f90:16 +! Linker invocation to generate the executable +! CHECK-LABEL: lld-link.exe +! CHECK-NOT: libcmt Meinersbur wrote: > This is failing for me. Instead of `lld-link.exe`,

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-31 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 433054. rovka added a comment. Updated test to check for link.exe instead of lld-link.exe. This doesn't look great but it works for both lld-link.exe and the default link.exe. I tried to use --ld-path=various/paths instead, but it seems to be ignored on Window

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-27 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3536718 , @kiranchandramohan wrote: > @rovka in case you missed this, the test is failing in windows. Yeah, thanks, I was out of office a bit so I couldn't get to it sooner. I'll give it a try without lld. Repository

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-26 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added inline comments. Comment at: flang/test/Driver/flang-linker-flags-windows.f90:16 +! Linker invocation to generate the executable +! CHECK-LABEL: lld-link.exe +! CHECK-NOT: libcmt This is failing for me. Instead of `lld-link.exe`, it is using `li

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-25 Thread Kiran Chandramohan via Phabricator via cfe-commits
kiranchandramohan added a comment. @rovka in case you missed this, the test is failing in windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 ___ cfe-commi

[PATCH] D126291: [flang][Driver] Update link job on windows

2022-05-24 Thread Diana Picus via Phabricator via cfe-commits
rovka created this revision. rovka added reviewers: awarzynski, kiranchandramohan. rovka added a project: Flang. Herald added a subscriber: jdoerfert. Herald added a reviewer: sscalpone. Herald added a project: All. rovka requested review of this revision. Herald added subscribers: cfe-commits, Mas