Adds mingw64 to the valid GHC OSs.
ClosedPublic

Authored by angerman on Sep 24 2017, 8:09 AM.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
angerman created this revision.Sep 24 2017, 8:09 AM
bgamari edited edge metadata.

What changed to introduce this breakage? I'm hesitant to merge this until we know what is going on as this used to work.

This was introduced in D4004

specifically the LLVM_TARGET is now canonicalized via the GHC_ macros, and hence the GHC_CONVERT_OS falls over for mingw64. For mingw32 we used to have this anyway already.

Phyx edited edge metadata.Sep 24 2017, 8:35 AM

I too wonder what happened here. I saw the failures on Harbormaster but locally it's working.

I guess this only happens if you pass in --target as some *-*-mingw64?

Phyx added a comment.Sep 24 2017, 8:42 AM

My guess is it's passing the unnormalized target around now. Previously we would use the inferred target

checking build system type... x86_64-pc-mingw64
checking host system type... x86_64-pc-mingw64
checking target system type... x86_64-pc-mingw64
Build platform inferred as: x86_64-unknown-mingw32
Host platform inferred as: x86_64-unknown-mingw32
Target platform inferred as: x86_64-unknown-mingw32
Unknown OS mingw64

So we'd pass x86_64-unknown-mingw32. Now it seems the original target is passed/used x86_64-pc-mingw64.

I don't think this is the right fix, as all our code assumes the mingw32 runtime, so configure creates mingw32_HOST_OS, if I'm not mistaken, isn't mingw64_HOST_OS being created instead now?

This is *new*, because we now use this for the LLMV_TARGET, which we previously passed verbatim; as this caused some issues with gentoos funny *-hardfloat-* vendors hack for hard float abis.

As we construct the LLVM_TARGET unconditionally, we now run into this.

I forgot to mention, the LLVM_TARGET is purely used (so far) for the lookup of the corresponding entry in the llvm-targets file (which interestedly doesn't even contain any mingw entries so far -- though this could be changed trivially).

Phyx added a comment.Sep 24 2017, 9:36 AM

Right, I see, so previously GHC_CONVERT_OS wouldn't be called for Windows when not cross compiling.

llvm only checks for mingw, win32 and windows in the os string, it doesn't care about the specific runtime since it's all using the Microsoft one anyway, so instead, so we don't run into problems when
someone mistakenly does --target=x86_64-w64-mingw64 I would instead map mingw64 to mingw32.

In D4016#112371, @Phyx wrote:

Right, I see, so previously GHC_CONVERT_OS wouldn't be called for Windows when not cross compiling.

llvm only checks for mingw, win32 and windows in the os string, it doesn't care about the specific runtime since it's all using the Microsoft one anyway, so instead, so we don't run into problems when
someone mistakenly does --target=x86_64-w64-mingw64 I would instead map mingw64 to mingw32.

I'm not entirely excited about this; I won't object, but I personally find it quite confusing when mingw32 I see on a 64-bit machine.

Phyx added a comment.Sep 24 2017, 1:52 PM
In D4016#112371, @Phyx wrote:

Right, I see, so previously GHC_CONVERT_OS wouldn't be called for Windows when not cross compiling.

llvm only checks for mingw, win32 and windows in the os string, it doesn't care about the specific runtime since it's all using the Microsoft one anyway, so instead, so we don't run into problems when
someone mistakenly does --target=x86_64-w64-mingw64 I would instead map mingw64 to mingw32.

I'm not entirely excited about this; I won't object, but I personally find it quite confusing when mingw32 I see on a 64-bit machine.

But mingw64 would be equally confusing. The naming of the os part of the target triple is a bit unfortunate, but 32-bit mingw-w64 GCC is also mingw64.

The target triple is important for GCC to determine the backend to use and thus which runtime ultimately ends up getting used. The mingw64 only means to use
the mingw64 backend, not that it's 32 or 64 bit.

For us, the problem is allowing mingw64 would break GHC compilation if you use the same target triple as GCC would expect (which is reasonable). With this change
I suspect cross compiling to --target=x86_64-w64-mingw64 would generate mingw64_HOST_OS and since we have no code path for that I have no idea what would happen.

Ideally, this confusion could be removed by normalizing the windows builds to win32 like clang so you know which subsystem it targets, or maybe just windows.

angerman updated this revision to Diff 14085.Sep 24 2017, 7:35 PM
  • catch windows in GHC_LLVM_TARGET, instead of delegating to GHC_CONVERT_OS

Maybe this is a better approach. To catch the mingw32 and mingw64 in the GHC_LLVM_TARGET macro, and convert them to windows, which is what is listed in the llvm-targets file right now anyway.

Phyx accepted this revision.Sep 25 2017, 2:05 AM

Yes, that's a better way. Thanks!

This revision is now accepted and ready to land.Sep 25 2017, 2:05 AM
This revision was automatically updated to reflect the committed changes.