Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
On 03/03/21 19:34 +0200, Ville Voutilainen wrote: On Wed, 3 Mar 2021 at 19:25, Jonathan Wakely via Libstdc++ wrote: Oh, except that is_scalar is surprisingly expensive to instantiate (its defined in a really expensive way) and since we control all uses I'll be more than happy to write you an __is_scalar for GCC 12. :P That would help with https://gcc.gnu.org/PR96710 too. The current implementation is inefficient and arguably wrong.
Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
On Wed, 3 Mar 2021 at 19:25, Jonathan Wakely via Libstdc++ wrote: > Oh, except that is_scalar is surprisingly expensive to instantiate > (its defined in a really expensive way) and since we control all uses I'll be more than happy to write you an __is_scalar for GCC 12. :P
Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
On Wednesday, 3 March 2021 08:21:51 PST Jonathan Wakely wrote: > >>- = is_same_v, __platform_wait_t>; > >>+ = is_scalar_v> && sizeof(_Tp) == > >>sizeof(__platform_wait_t) > Oh, except that is_scalar is surprisingly expensive to instantiate > (its defined in a really expensive way) and since we control all uses > of this constant, I don't think we need it. It's only ever used from > atomic waiting functions which are only defined for scalar types. Thanks. Will update and re-submit. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel DPG Cloud Engineering
Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
On 03/03/21 14:34 +, Jonathan Wakely wrote: On 26/02/21 07:59 -0800, Thiago Macieira via Libstdc++ wrote: The kernel doesn't care what we store in those 32 bits, only that they are comparable. This commit adds: * pointers and long on 32-bit architectures * unsigned * untyped enums and typed enums on int & unsigned int * float We're not using FUTEX_OP anywhere today. The kernel reserves 4 bits for this field but has only used 6 values so far, so it can be extended to unsigned compares in the future, if needed. And this one for GCC 11 too. libstdc++-v3/include/bits/atomic_wait.h | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_wait.h b/libstdc++-v3/include/bits/atomic_wait.h index 424fccbe4c5..1c6bda2e2b6 100644 --- a/libstdc++-v3/include/bits/atomic_wait.h +++ b/libstdc++-v3/include/bits/atomic_wait.h @@ -65,7 +65,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template inline constexpr bool __platform_wait_uses_type #ifdef _GLIBCXX_HAVE_LINUX_FUTEX - = is_same_v, __platform_wait_t>; + = is_scalar_v> && sizeof(_Tp) == sizeof(__platform_wait_t) Oh, except that is_scalar is surprisingly expensive to instantiate (its defined in a really expensive way) and since we control all uses of this constant, I don't think we need it. It's only ever used from atomic waiting functions which are only defined for scalar types. + && alignof(_Tp) >= alignof(__platform_wait_t); #else = false; #endif @@ -91,13 +92,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template void - __platform_wait(const _Tp* __addr, __platform_wait_t __val) noexcept + __platform_wait(const _Tp* __addr, _Tp __val) noexcept { for(;;) { auto __e = syscall (SYS_futex, static_cast(__addr), static_cast(__futex_wait_flags::__wait_private), - __val, nullptr); + static_cast<__platform_wait_t>(__val), nullptr); if (!__e || errno == EAGAIN) break; else if (errno != EINTR) -- 2.30.1
Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
On 26/02/21 07:59 -0800, Thiago Macieira via Libstdc++ wrote: The kernel doesn't care what we store in those 32 bits, only that they are comparable. This commit adds: * pointers and long on 32-bit architectures * unsigned * untyped enums and typed enums on int & unsigned int * float We're not using FUTEX_OP anywhere today. The kernel reserves 4 bits for this field but has only used 6 values so far, so it can be extended to unsigned compares in the future, if needed. And this one for GCC 11 too. libstdc++-v3/include/bits/atomic_wait.h | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_wait.h b/libstdc++-v3/include/bits/atomic_wait.h index 424fccbe4c5..1c6bda2e2b6 100644 --- a/libstdc++-v3/include/bits/atomic_wait.h +++ b/libstdc++-v3/include/bits/atomic_wait.h @@ -65,7 +65,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template inline constexpr bool __platform_wait_uses_type #ifdef _GLIBCXX_HAVE_LINUX_FUTEX - = is_same_v, __platform_wait_t>; + = is_scalar_v> && sizeof(_Tp) == sizeof(__platform_wait_t) + && alignof(_Tp) >= alignof(__platform_wait_t); #else = false; #endif @@ -91,13 +92,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template void - __platform_wait(const _Tp* __addr, __platform_wait_t __val) noexcept + __platform_wait(const _Tp* __addr, _Tp __val) noexcept { for(;;) { auto __e = syscall (SYS_futex, static_cast(__addr), static_cast(__futex_wait_flags::__wait_private), - __val, nullptr); + static_cast<__platform_wait_t>(__val), nullptr); if (!__e || errno == EAGAIN) break; else if (errno != EINTR) -- 2.30.1
[PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
The kernel doesn't care what we store in those 32 bits, only that they are comparable. This commit adds: * pointers and long on 32-bit architectures * unsigned * untyped enums and typed enums on int & unsigned int * float We're not using FUTEX_OP anywhere today. The kernel reserves 4 bits for this field but has only used 6 values so far, so it can be extended to unsigned compares in the future, if needed. --- libstdc++-v3/include/bits/atomic_wait.h | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_wait.h b/libstdc++-v3/include/bits/atomic_wait.h index 424fccbe4c5..1c6bda2e2b6 100644 --- a/libstdc++-v3/include/bits/atomic_wait.h +++ b/libstdc++-v3/include/bits/atomic_wait.h @@ -65,7 +65,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template inline constexpr bool __platform_wait_uses_type #ifdef _GLIBCXX_HAVE_LINUX_FUTEX - = is_same_v, __platform_wait_t>; + = is_scalar_v> && sizeof(_Tp) == sizeof(__platform_wait_t) + && alignof(_Tp) >= alignof(__platform_wait_t); #else = false; #endif @@ -91,13 +92,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template void - __platform_wait(const _Tp* __addr, __platform_wait_t __val) noexcept + __platform_wait(const _Tp* __addr, _Tp __val) noexcept { for(;;) { auto __e = syscall (SYS_futex, static_cast(__addr), static_cast(__futex_wait_flags::__wait_private), - __val, nullptr); + static_cast<__platform_wait_t>(__val), nullptr); if (!__e || errno == EAGAIN) break; else if (errno != EINTR) -- 2.30.1