MINGW-packages icon indicating copy to clipboard operation
MINGW-packages copied to clipboard

Should we migrate gcc from winpthreads to mcfgthread?

Open lhmouse opened this issue 1 year ago • 9 comments

I am speaking for myself, not for the mingw-w64 project.

Recently, as usual however, there was a report about deadlocks in winpthreads ^1. While I believe people are sincere and professional, some of these are not reproducible, and are presumed to have been fixed by a fragile patch ^2.

The threading library is so fundamental that, although not being 100% necessary, I think it's time to start replacing winpthreads with something more modern, such as mcfgthread ^3.

The pros of winpthreads are:

  1. winpthreads supports Windows XP; mcfgthread only supports Vista.
  2. winpthreads provides more complete POSIX APIs, such scheduler and RW locks; mcfgthread only provides those required by libgcc and libstdc++.

On the other hand, the pros of mcfgthread are

  1. mcfgthread is generally more efficient, outperforming native SRWLOCKs; winpthreads' condition variable and thread-specific storage access is slow.
  2. mcfgthread provides slim mutex and condvar, which take up storage as pointers, consume no additional resource, and require no cleanup.
  3. mcfgthread provides __cxa_finalize() as per Itanium ABI.

lhmouse avatar Aug 12 '22 06:08 lhmouse

2. winpthreads provides more complete POSIX APIs, such scheduler and RW locks; mcfgthread only provides those required by libgcc and libstdc++.

  • Is this a design limitation, or "just" needs more work?

Other questions:

  • Can/could winpthreads and mcfgthread coexist in MSYS2?
  • Is it just a replacement for gcc/libstdc++, or should/could other things also migrate to it?

lazka avatar Aug 12 '22 08:08 lazka

Oops, sorry, I mistook this repo for MINGW-packages. Please move it there.

MSYS2 itself uses Cygwin's pthread library I think? There should be no issue.

lhmouse avatar Aug 12 '22 08:08 lhmouse

MSYS2 itself uses Cygwin's pthread library I think? There should be no issue.

I think this, yes: https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/thread.cc

lazka avatar Aug 12 '22 08:08 lazka

  1. winpthreads provides more complete POSIX APIs, such scheduler and RW locks; mcfgthread only provides those required by libgcc and libstdc++.
  • Is this a design limitation, or "just" needs more work?

It's mostly the latter.

Scheduling APIs are fairly easy to implement, as they are just wrappers for Windows APIs, while clock and the semaphore APIs are a bit more complex. The POSIX semaphore can be named or unnamed, so probably we can have sem_t as a struct holding solely the handle to a Windows semaphore. At least, it's no magic.

Other questions:

  • Can/could winpthreads and mcfgthread coexist in MSYS2?
  • Is it just a replacement for gcc/libstdc++, or should/could other things also migrate to it?

In principle they can. Not sure whether it is a good idea, though, as some configure scripts attempt to check whether linking against pthread succeeds or not, and I am not sure whether they would interfere with each other. Anyway, code that calls pthread APIs explicitly, instead of referencing them via the C++ standard library, is unaffected.

Existent programs and libraries that use C++11 threads used to link against winpthreads, so they should continue to work. However, once rebuilt, they shouldn't depend on winpthreads any more.

lhmouse avatar Aug 12 '22 08:08 lhmouse

ok, I see, I was initially thinking this is a winpthread alternative, but it is a gthread alternative, this my questions didn't make much sense :)

Some more questions:

  • Do you see any problems with existing projects dynamically or statically linking libstdc++ and assuming pthreads being used?

  • Would it makes sense to upstream the gthread implementation into gcc itself, so there is no extra library needed, similar to libc++? Or patch it in and build it together.

lazka avatar Aug 12 '22 13:08 lazka

  • Do you see any problems with existing projects dynamically or statically linking libstdc++ and assuming pthreads being used?

No. The only issue that I am aware of is that one may assume std::thread::native_handle() returns a pthread_t and passes it to other pthread functions; however I haven't seen anyone do this.

  • Would it makes sense to upstream the gthread implementation into gcc itself, so there is no extra library needed, similar to libc++? Or patch it in and build it together.

While it may simplify distribution of such supplementary libraries, this library depends on no details about GCC - unlike libssp, libquadmath, etc.; even libstdc++ depends on some GCC builtin functions - and is not to be maintained by GCC developers. I am not denying the possibility though, I am a bit negative whether they will have any interest.

lhmouse avatar Aug 12 '22 13:08 lhmouse

well from personal experience i can say mcfgthread works very well with gcc :) the only downside im aware of is that it wont work on anything older than Win vista so thats pretty moot.

revelator avatar Aug 16 '22 19:08 revelator

well from personal experience i can say mcfgthread works very well with gcc :) the only downside im aware of is that it wont work on anything older than Win vista so thats pretty moot.

Thanks for the feedback.

MSYS2 has been ~~Vista-only~~ Vista-or-above-only for many years (and will be ~~Windows 8 only~~ Windows-8.1-or-above-only lately this year) so I think mcfgthread fits here.

lhmouse avatar Aug 17 '22 01:08 lhmouse

Might need to clarify Vista and above only rather than Vista-only, I got confused for a moment

1480c1 avatar Aug 17 '22 02:08 1480c1

thats why i ment the point was rather moot ;)

revelator avatar Aug 20 '22 21:08 revelator

one strange problem turned up when trying to port gcc to mcfgthread the mcfgthread gcc bootstrap seems to do something odd to __cpuid erroring out because it only detects __cpuid as having 2 variables instead of the 5 needed in libstdc++ random.cc so i cannot build it as is. also theres a strange error with the latest crt complaining about TLS failure, this seems to be due to a recently added patch by martin storsjoe in the mingw-w64 abi.

revelator avatar Aug 22 '22 20:08 revelator

one strange problem turned up when trying to port gcc to mcfgthread the mcfgthread gcc bootstrap seems to do something odd to __cpuid erroring out because it only detects __cpuid as having 2 variables instead of the 5 needed in libstdc++ random.cc so i cannot build it as is. also theres a strange error with the latest crt complaining about TLS failure, this seems to be due to a recently added patch by martin storsjoe in the mingw-w64 abi.

May I ask which GCC version you are trying to bootstrap? I have been building GCC 12 for a few months and have not seen issues around __cpuid so far. I will perform a new build today and see whether the problem persists.

mcfgthread can work with the vanilla mingw-w64 CRT or patched CRT (so exit() etc. work as specified by standards). Replacing the CRT requires a cross-compilation setup. I think I should have documented this; here are some brief instructions, but I can copy these elsewhere if that's desired:

  1. Install vanilla toolchains: pacman -S mingw-w64-{i686,x86_64,ucrt-x86_64}-gcc-git
  2. Apply patches for GCC and mingw-w64 CRT.
  3. Build & install mingw-w64 headers.
  4. Build & install mingw-w64-gcc with --enable-threads=mcf --disable-bootstrap. This creates a compiler that runs on the vanilla CRT, but targets the patched CRT.
  5. Build & install mingw-w64 patched CRT.
  6. Rebuild & install GCC with --enable-bootstrap.

lhmouse avatar Aug 23 '22 01:08 lhmouse

It looks virtually OK if older versions of Windows are not supported. One more thing: native_handle is implementation-defined, so there should be sufficient documents for the change, whether it is upstreamed or not.

FrankHB avatar Aug 23 '22 02:08 FrankHB

It looks virtually OK if older versions of Windows are not supported. One more thing: native_handle is implementation-defined, so there should be sufficient documents for the change, whether it is upstreamed or not.

The native_handle_type type is an alias for __gthread_t, which is an alias for struct __MCF_thread* defined in 'thread.h'. Specifically, native_handle() now returns a __MCF_thread*. One can't assume that it is a pthread_t or Windows HANDLE.

lhmouse avatar Aug 23 '22 04:08 lhmouse

I have successfully bootstrapped GCC 12.2.1 (release branch) and observed no issue so far.

  • Source: https://github.com/lhmouse/MINGW-packages/tree/master_2022-08-23
  • Binary: https://gcc-mcf.lhmouse.com/

lhmouse avatar Aug 23 '22 05:08 lhmouse

might be because im using an older pthread build as bootstrap compiler for it maybe ?, else not sure ?.

revelator avatar Aug 23 '22 08:08 revelator

might be because im using an older pthread build as bootstrap compiler for it maybe ?, else not sure ?.

That shouldn't affect the build itself. There may be some other issues.

Details explained:

  1. Patches are applied to GCC and the CRT, and don't affect the headers.
  2. GCC itself doesn't use thread-local storage, only GCC libraries do, so the __cpuid thing looks effected by something else. (maybe this?) Anyway, upgrading GCC is recommended.
  3. The CRT may depend on a threading library. This requires we build and install the threading library before the CRT. winpthreads has dummy libraries for libgcc etc., while mcfgthread is built with -nostdlib, so neither depends back on the CRT (otherwise we get unsolvable circular dependencies).
  4. The final bootstrap is required so GCC libraries are linked against the patched CRT.

lhmouse avatar Aug 23 '22 08:08 lhmouse

no 2 sounds plausible, guess ill have to patch that out while bootstrapping ill try again later.

revelator avatar Aug 23 '22 10:08 revelator

gcc-11.3.0 was the version, havent gotten around to rebuilding it yet though.

revelator avatar Aug 24 '22 07:08 revelator

gcc-11.3.0 was the version, havent gotten around to rebuilding it yet though.

Probably you have to update mingw-w64 headers first.

lhmouse avatar Aug 24 '22 09:08 lhmouse

i did :)

revelator avatar Aug 24 '22 13:08 revelator

Did you try bootstrapping mingw-w64-gcc from this repo, without any other patches? As this is the official MSYS2 source, it should build without errors.

lhmouse avatar Aug 24 '22 14:08 lhmouse

aye and without the mcfgthread patches it builds fine so i was a bit stumped. the problem seems to be in the crt though one error related to tls and one with cpuid.

revelator avatar Aug 24 '22 14:08 revelator

ill try reverting to an older mingw-w64-abi to see what happens.

revelator avatar Aug 24 '22 14:08 revelator