Also properly declare intrinsic on msvc.
Signed-off-by: Jacek Caban [email protected] --- dlls/ntdll/rtl.c | 26 ++++---------------------- include/winnt.h | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 22 deletions(-)
There's no __sync_bool_compare_and_swap_16 in ARMv8.0 (or even with ARMv8.1, support wasn't added until until GCC 10).
This of course broke our builds when it was added; for now I'd been working around it with the attached patch, which substitutes the -latomic __atomic_compare_exchange_16, which is allowed to be based on a lock if there's no lockfree implementation (e.g. the specified -march or GCC lack support for the ARMv8.1 CASP* instructions).
I was waiting to hear back from our BSP guys whether we'd eventually get support on the final hardware/toolchain or not before raising this upstream, to see know if I had an issue that needed a "real" fix or just a short-term impediment (in which case I had a viable workaround in the meantime).
But if you're moving it into external API too, then it seems like it's time to at least point out what I've learned.
Microsoft only promises InterlockedCompareExchange128 on amd64; It's nice to offer it anywhere an underlying compiler intrinsic exists, but that needs to test defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16). And I think RtlInterlocked*SList needs some kind of fallback. What I did with -latomic seems sane, but should get a ./configure check since you wouldn't need to link -latomic if the compiler has the intrinsic.
-----Original Message----- From: wine-devel [email protected] On Behalf Of Jacek Caban Sent: Friday, July 31, 2020 8:38 AM To: wine-devel [email protected] Subject: [EXTERNAL] [PATCH 2/2] ntdll: Move InterlockedCompareExchange128 support to winnt.h.
[EXTERNAL EMAIL]: This message was generated from an external source. Do not follow guidance, click links, or open attachments unless you recognize the sender and know the content is safe.
Also properly declare intrinsic on msvc.
Signed-off-by: Jacek Caban [email protected]
dlls/ntdll/rtl.c | 26 ++++---------------------- include/winnt.h | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 22 deletions(-)
On 31.07.2020 17:01, Puetz Kevin A wrote:
There's no __sync_bool_compare_and_swap_16 in ARMv8.0 (or even with ARMv8.1, support wasn't added until until GCC 10).
This of course broke our builds when it was added; for now I'd been working around it with the attached patch, which substitutes the -latomic __atomic_compare_exchange_16, which is allowed to be based on a lock if there's no lockfree implementation (e.g. the specified -march or GCC lack support for the ARMv8.1 CASP* instructions).
I was waiting to hear back from our BSP guys whether we'd eventually get support on the final hardware/toolchain or not before raising this upstream, to see know if I had an issue that needed a "real" fix or just a short-term impediment (in which case I had a viable workaround in the meantime).
But if you're moving it into external API too, then it seems like it's time to at least point out what I've learned.
Microsoft only promises InterlockedCompareExchange128 on amd64; It's nice to offer it anywhere an underlying compiler intrinsic exists, but that needs to test defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16). And I think RtlInterlocked*SList needs some kind of fallback. What I did with -latomic seems sane, but should get a ./configure check since you wouldn't need to link -latomic if the compiler has the intrinsic.
Oh, I see, I didn't know it's already broken on aarch64. FWIW, I just wanted to fix a warning on clang msvcrt target and while doing that, I generalized the solution. We can wait with that for aarch64 solution.
Depending on -latomic may be problematic in PE builds. If I read libatomic source right, libatomic resorts to using lock in this case (pthread_mutex_lock or equivalent, depending on host). We could probably just add a variant of affected RtlInterlocked*SList* functions that would use critical section instead of interlocked operations.
Thanks,
Jacek
-
Jacek Caban -
Puetz Kevin A