Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int

[Jonathan Wakely via Gcc-patches]

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

[Ville Voutilainen via Gcc-patches]

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

[Thiago Macieira via Gcc-patches]

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

[Jonathan Wakely via Gcc-patches]

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

[Jonathan Wakely via Gcc-patches]

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