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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D126291#3562471 <https://reviews.llvm.org/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 compiler that does not have support for C++17.
>
> But re-reading this again, I might have mis-read the "and" for an "or"...

Don't read too much into it - it used to be the case that flang wasn't tested/supported at all on Windows, but at this point I think this comment just is outdated/leftovr.



================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:140
+    // defined in flang's runtime libraries.
+    if (TC.getTriple().isKnownWindowsMSVCEnvironment())
+      CmdArgs.push_back("/subsystem:console");
----------------
mmuetzel wrote:
> Meinersbur wrote:
> > mmuetzel wrote:
> > > awarzynski wrote:
> > > > mmuetzel wrote:
> > > > > Is the MSVC toolchain used anywhere else but on Windows?
> > > > No: https://github.com/llvm/llvm-project/blob/5fee1799f4d8da59c251e2d04172fc2f387cbe54/llvm/include/llvm/ADT/Triple.h#L576-L578 ;-) 
> > > In that case, this argument could probably be added unconditionally.
> > How do you mean this? `/subsystem:console` is a `link.exe` flag, also supported by `lld-link.exe` sind it tries to be an msvc-compatible wrapper. It is not a valid flag for eg GNU ld even in msys.
> Sorry my questions weren't very clear. What I meant to ask:
> The name of this file is `ToolChains/MSVC.cpp`. I was wondering whether that means that these settings are only relevant when using a MSVC (compatible) toolchain. If that is the case, checking for `isKnownWindowsMSVCEnvironment()` here might be redundant. At least, I can't find that being checked anywhere else in this file.
Yes, you can assume `isKnownWindowsMSVCEnvironment()` within ToolChains/MSVC.cpp, so link.exe style linker options like `/subsystem:console` can be added unconditionally.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126291/new/

https://reviews.llvm.org/D126291