[PATCH] D119332: [flang] Add Win32 to the list of supported triples

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added a comment.

In D119332#3311785 <https://reviews.llvm.org/D119332#3311785>, @schweitz wrote:

> I don't follow that.
>
> The KindMap is not related to the target machine. It is effectively a mini-language to allow support for different dialects of Fortran that may use or reassign KIND values (in the source syntax) as a form of future-proofing the optimizer. (It's a source code thing.)

@schweitz, I referred to `KindMap` as that's one of the things that's returned by  `fir::CodeGenSpecifics::get`. I think that we agree that it's rather OS-agnostic and in this respect, this change is safe.

> Code gen to a specific target can and does depend on what the target (triple) is from the user, the ABI(s) of that target, and how the LLVM machine code expects to see aspects of the LLVM IR presented to produce the correct calls, memory layouts, etc. flang has to follow in clang's footsteps in that regard as different LLVM backends expect different types, etc. (The target is a codegen thing.)

Noted. This is currently expressed via e.g TargetI386 <https://github.com/flang-compiler/f18-llvm-project/blob/7205bde0047eaf4aae812f6fc5c0780e39c0f058/flang/lib/Optimizer/CodeGen/Target.cpp#L76-L115> or TargetAArch64 <https://github.com/flang-compiler/f18-llvm-project/blob/7205bde0047eaf4aae812f6fc5c0780e39c0f058/flang/lib/Optimizer/CodeGen/Target.cpp#L171_L205>, right? These structs contain **architecture-specific** information, but I don't see anything **OS-specific**. For this reason, I feel that it is safe to enable `Win32` here. This way:

- we can keep our tests more generic (i.e. avoid using `--target` when the generated code is target-agnostic),
- we avoid reducing the test coverage on Windows.

I think that we all agree that more work will be required to make sure that all ABIs work as expected. But we can re-visit this in due time and fine-tune things as required. For now, basic tests on Windows (and MacOSX <https://github.com/flang-compiler/f18-llvm-project/runs/5149548747?check_suite_focus=true>, but that's on fir-dev) hit llvm::report_fatal_error <https://github.com/flang-compiler/f18-llvm-project/blob/7205bde0047eaf4aae812f6fc5c0780e39c0f058/flang/lib/Optimizer/CodeGen/Target.cpp#L290> unless the triple is hard-coded. But we run our CI on these platforms, `fir::CodeGenSpecifics::get` is OS-agnostic (am I missing something here?), so why not include `Win32` (and MacOSX <https://github.com/flang-compiler/f18-llvm-project/pull/1473/commits/d43181e452f5df0bf83b644946f16f2785f92303>) here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119332