Page MenuHomePhabricator

enable -fms-extensions by default on the mingw-w64 target
Needs RevisionPublic

Authored by martell on Oct 29 2015, 10:19 AM.

Details

Reviewers
compnerd
rnk
Summary

As of commit b8a164 mingw-w64 support clang with -fms-extensions.
We can built the mingw-w64 crt with clang now also.
As we are dropping support for mingw.org I think switching to this would be a good move
rather then using the gcc hacks we currently comply to

http://sourceforge.net/p/mingw-w64/mingw-w64/ci/b8a16418409d88215cce97727a42cc25eb011f3e/

Diff Detail

Event Timeline

martell updated this revision to Diff 38746.Oct 29 2015, 10:19 AM
martell retitled this revision from to enable -fms-extensions by default on the mingw-w64 target.
martell updated this object.
martell added a reviewer: rnk.
martell added subscribers: cfe-commits, yaron.keren, compnerd.
compnerd requested changes to this revision.Oct 29 2015, 9:54 PM
compnerd added a reviewer: compnerd.

Please add a unit test for this.

This revision now requires changes to proceed.Oct 29 2015, 9:54 PM
rnk edited edge metadata.Nov 2 2015, 4:30 PM

Will Clang be able to compile the last few stable mingw-w64 releases with this change? I'd like that to keep working.

In D14180#279651, @rnk wrote:

Will Clang be able to compile the last few stable mingw-w64 releases with this change? I'd like that to keep working.

Clang has not been able to compile mingw-w64 crt for the last few releases.
I have only added support for this on HEAD.

There would be a drawback though.
Clang would not like older mingw-w64 headers.

rnk added a comment.Nov 3 2015, 9:16 AM
In D14180#279651, @rnk wrote:

Will Clang be able to compile the last few stable mingw-w64 releases with this change? I'd like that to keep working.

Clang has not been able to compile mingw-w64 crt for the last few releases.
I have only added support for this on HEAD.

Right, I'm not worried about that.

There would be a drawback though.
Clang would not like older mingw-w64 headers.

This is what I'm worried about. :) Can you elaborate on what would break?

This is what I'm worried about. :) Can you elaborate on what would break?

Essentially mingw-w64 has a few header defines for working around builtings such as __rtdsc and the interlocked functions and some other builtins
As soon as one of these headers are included it would result in a compile time error as it would collide with what clang now provides.

I have talked to jon_y the 4.x maintainer about this before.
We could port back my patches to next 4.x release which i believe is 4.0.4.
But essentially older versions of mingw-w64 that doesn't respect the compiler may have the builtins would not work.

We don't have to make this break now of course but we should do it at some point?

rnk added a comment.Nov 3 2015, 9:49 AM

I'd like clang to work out of the box with mingw-w64 releases from at least the past year. You're mostly interested in the non-builtin parts of -fms-extensions, like declspec and UUID, right? One way we could do this is to enable -fms-extensions in mingw but then suppress the builtins in that environment, or have some other flag to enable them.

You can then change the mingw headers to use __has_builtin(_InterlockedCompareExchange) to provide compatibility with either mode.

In D14180#280184, @rnk wrote:

You're mostly interested in the non-builtin parts of -fms-extensions, like declspec and UUID, right?

Yes that would be one of the main reasons, gcc is very lacking in this department :)

You can then change the mingw headers to use __has_builtin(_InterlockedCompareExchange) to provide compatibility with either mode.

Yes I could do this but this still won't work with the the existing mingw toolchains that don't have this in their headers right?
I can update head to support both though yes

chapuni added a subscriber: chapuni.Nov 3 2015, 2:07 PM
rnk added a comment.Nov 4 2015, 3:33 PM

! In D14180#280193, @martell wrote:

You can then change the mingw headers to use __has_builtin(_InterlockedCompareExchange) to provide compatibility with either mode.

Yes I could do this but this still won't work with the the existing mingw toolchains that don't have this in their headers right?
I can update head to support both though yes

I'm suggesting that we can make the builtins available in an MSVC environment, rather than making them available in an MS extensions environment. This should be compatible with old mingw headers, and then we can flip this default.

I tried testing __has_builtin(_InterlockedCompareExchangeAdd) as it seem to not exist even when -fms-extensions is passed
Is this a bug or it is intended ?

rnk added a comment.Dec 1 2015, 9:45 AM

I tried testing __has_builtin(_InterlockedCompareExchangeAdd) as it seem to not exist even when -fms-extensions is passed
Is this a bug or it is intended ?

Neither Clang nor MSDN have anything by that name. Do you mean _InterlockedExchangeAdd? __has_builtin should find that.

@rnk you are correct.
mingw-w64 has been updated accordingly
http://sourceforge.net/p/mingw-w64/mingw-w64/ci/61c374ded91a7de66f42f85a9f0cf9ee20/

So where do we go from here ?

Kind Regards
Martell

rnk added a comment.Dec 3 2015, 5:49 PM

@rnk you are correct.
mingw-w64 has been updated accordingly
http://sourceforge.net/p/mingw-w64/mingw-w64/ci/61c374ded91a7de66f42f85a9f0cf9ee20/

So where do we go from here ?

Let's disable the _Interlocked* family of builtins when targeting mingw, and then flip this flag here. That way we'll work with old mingw's, and in two years we can enable the builtins in mingw. We'll also let mingw get at the rest of -fms-extensions.