Switching to the new MingW ABI

Mingw switched abis with the release of gcc 4.7
(GCC 4.7 Release Series — Changes, New Features, and Fixes - GNU Project). The main change is that now
mingw (like msvc) given thiscall calling convention to methods by
default.

I think the last bug blocking us to support the new abi has just been
fixed. The question now is how to switch.

The attached patches simply switch llvm and clang to the new ABI. This
is similar to what gcc did on 4.7. The timing is also good as we will
not build with 4.6 anymore when we switch to c++11.

Is anyone depending on targeting the 4.6 mingw ABI even after we drop
support for building with 4.6?

Cheers,
Rafael

clang.patch (1.07 KB)

llvm.patch (3.49 KB)

It’s worth noting that gcc chose not to support any ABI changing flags.

I’m in favor of avoiding flags here. We can simply document that clang 3.3 and earlier works with gcc 4.6 and earlier, and clang 3.4+ works with gcc 4.7+.

I’m with Reid.

Yaron

+1.

In case of ARM e.g. we can make sure we won't link stuff with
incompatible ABIs, but not here....

For the current project I’m working on (https://github.com/mono/CppSharp) having the flags to change the ABI based on a GCC version would be ideal. If there are no flags, this means we must implement some logic to change the calling conventions of methods manually to how they were pre-4.7. Should not be a lot of work but it’d be best to contain all the C++ ABI details inside Clang itself.

It’s also likely the ABI will change in the future, so this would allow to support those changes while maintaining compatibility with earlier GCC versions.

By the way, we’ve been working around this MinGW ABI thiscall issue for a couple weeks so thanks for fixing it.

Because you must link with gcc 4.6 compiled libraries for now or is
that a permanent thing?

Cheers,
Rafael

The tool uses Clang to parse the user’s C/C++ code to get the calling conventions from the AST, so they need to match the ones in the compiled libraries to allow correct interop. If the user libraries were compiled with GCC 4.6 (which stills seems used by some MinGW distros) then once we upgrade to the latest Clang we’ll start getting thiscall CC instead of the correct one used pre-4.7 for those libraries. If there was something like an “-fgcc-abi=4.6” flag we could pass to Clang, then it would provide us with the correct CCs for the given GCC version.

If the community thinks that is undesirable for the project then we can work around it on our side, but it seems to me these details should be contained in Clang.

I'm still seeing some test failures. For example,
CXX/class.access/p6.cpp fails if I apply your patch and run with
-triple i686-pc-mingw32:

$ "D:/src/llvm/build.release/bin/./clang.EXE" "-cc1" -triple
i686-pc-mingw32 "-internal-isystem"
"D:\src\llvm\build.release\bin\..\lib\clang\3.4\include"
"-fsyntax-only" "-verify"
"D:\src\llvm\tools\clang\test\CXX\class.access\p6.cpp"
error: 'error' diagnostics expected but not seen:
  File D:\src\llvm\tools\clang\test\CXX\class.access\p6.cpp Line 180:
'operator void *(class test8::a::*)(void) const' is a private member
of 'test8::A'
error: 'error' diagnostics seen but not expected:
  File D:\src\llvm\tools\clang\test\CXX\class.access\p6.cpp Line 180:
'operator void *(class test8::a::*)(void) __attribute__((thiscall))
const' is a private member of 'test8::A'
2 errors generated.

There are a handful other tests failing in the same way.

It would be nice if we could make the TypePrinter not print the
calling convention if it's the default one for the ABI, but
TypePrinter doesn't have a lot of context.. no Sema, no ASTContext :confused:

- Hans

I’ll look into this. I think we should just print the unadjusted type if we have an adjusted function type under a member pointer. This should be an easy type printer fix.

There are a handful other tests failing in the same way.

It would be nice if we could make the TypePrinter not print the
calling convention if it's the default one for the ABI, but
TypePrinter doesn't have a lot of context.. no Sema, no ASTContext :confused:

They are all TypePrinter failures like this one? If so I would say we
should not block on it since it is a small QOI issue.

On the other hand Reid says it should be an easy fix, so we can wait a bit.

Cheers,
Rafael

The tool uses Clang to parse the user's C/C++ code to get the calling
conventions from the AST, so they need to match the ones in the compiled
libraries to allow correct interop. If the user libraries were compiled with
GCC 4.6 (which stills seems used by some MinGW distros)

Do you have an idea for how long you will have to support these
distros? This change would only show up on clang 3.5, and it is not
even expected to build with gcc 4.6

then once we upgrade
to the latest Clang we'll start getting thiscall CC instead of the correct
one used pre-4.7 for those libraries. If there was something like an
"-fgcc-abi=4.6" flag we could pass to Clang, then it would provide us with
the correct CCs for the given GCC version.

If the community thinks that is undesirable for the project then we can work
around it on our side, but it seems to me these details should be contained
in Clang.

On the clang side the change should be trivial (note the attached
clang.patch on first email). Do you think that is something you can
keep locally if needed?

If we can avoid creating new ABI options I think that is desirable.

Cheers,
Rafael

Yes, all except this one (I think):

error: 'error' diagnostics expected but not seen:
  File D:\src\llvm\tools\clang\test\SemaCXX\virtual-override.cpp Line
288: 'static' member function 'foo' overrides a virtual function
error: 'error' diagnostics seen but not expected:
  File D:\src\llvm\tools\clang\test\SemaCXX\virtual-override.cpp Line
288: virtual function 'foo' has different calling convention
attributes ('void ()') than the function it overrides (which has
calling convention 'void () __attribute__((thiscall))')
2 errors generated.

I guess this is also a QOI problem, but the diagnostic here gets
pretty misleading.

- Hans

Yes, all except this one (I think):

error: 'error' diagnostics expected but not seen:
  File D:\src\llvm\tools\clang\test\SemaCXX\virtual-override.cpp Line
288: 'static' member function 'foo' overrides a virtual function
error: 'error' diagnostics seen but not expected:
  File D:\src\llvm\tools\clang\test\SemaCXX\virtual-override.cpp Line
288: virtual function 'foo' has different calling convention
attributes ('void ()') than the function it overrides (which has
calling convention 'void () __attribute__((thiscall))')
2 errors generated.

I guess this is also a QOI problem, but the diagnostic here gets
pretty misleading.

Yes, I guess this a case of the error being pedantically correct. We
should suppress the second error.

Cheers,
Rafael

Something like http://llvm-reviews.chandlerc.com/D2375 ?

- Hans

Something like http://llvm-reviews.chandlerc.com/D2375 ?

Seems reasonable, but could the check in CheckFunctionDeclaration be
merged with this one so that we don't get duplicated code handling
diag::err_static_overrides_virtual?

Cheers,
Rafael

Does this patch fix any failures for you? It doesn't fix that test case,
unfortunately.

print-unadjusted-type.diff (1.77 KB)

Unfortunately, no, I still see the same failures. These are the
TypePrinter related failures I see:

    Clang :: CXX/class.access/p6.cpp
    Clang :: CXX/expr/expr.const/p3-0x.cpp
    Clang :: CXX/expr/expr.mptr.oper/p5.cpp
    Clang :: CXX/expr/expr.mptr.oper/p6-0x.cpp
    Clang :: CXX/expr/expr.unary/expr.unary.op/p4.cpp
    Clang :: CXX/temp/temp.arg/temp.arg.nontype/p5.cpp
    Clang :: SemaCXX/addr-of-overloaded-function.cpp
    Clang :: SemaCXX/const-cast.cpp
    Clang :: SemaCXX/cstyle-cast.cpp
    Clang :: SemaCXX/functional-cast.cpp
    Clang :: SemaCXX/reinterpret-cast.cpp
    Clang :: SemaCXX/static-cast.cpp
Printing thiscall attribute on member function pointer type.

    Clang :: CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
    Clang :: SemaTemplate/explicit-instantiation.cpp
    Clang :: SemaTemplate/instantiate-method.cpp
    Clang :: SemaTemplate/temp_arg_nontype.cpp
Printing thiscall attribute for function type after explicit
specialization function match failure.

- Hans

I think we need to relax the test cases. MSVC usually prints the calling convention, and it’s often useful information.

Maybe we can make the diagnostic text smaller by using the __thiscall, __cdecl, __stdcall, etc keywords when printing types with LangOpts.MicrosoftExt, but it will require moving the attribute from after the identifier to before, which is exciting.

I think we need to relax the test cases. MSVC usually prints the calling
convention, and it's often useful information.

I was going to argue that MSVC doesn't print them, but then I tried,
and you're right - it does :slight_smile:

I'll start relaxing the tests then.

Maybe we can make the diagnostic text smaller by using the __thiscall,
__cdecl, __stdcall, etc keywords when printing types with
LangOpts.MicrosoftExt, but it will require moving the attribute from after
the identifier to before, which is exciting.

This seems like a nice thing to do, but I'll get the tests going with
what we've got for now.

- Hans

Maybe we should also change the stack probing code. I think ___chkstk_ms is used since gcc 4.6. It simplifies the prologue generation code a bit.

Regards,
Kai

mingw.diff (3.2 KB)