public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934]
@ 2022-01-15  2:53 Thomas Rodgers
  2022-02-02 21:35 ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Rodgers @ 2022-01-15  2:53 UTC (permalink / raw)
  To: libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0001-libstdc-Add-missing-free-functions-for-atomic_flag-P.patch --]
[-- Type: text/x-patch, Size: 3267 bytes --]

From c2b74fd7cf2668d288f46da42565e5eb954e5e1f Mon Sep 17 00:00:00 2001
From: Thomas Rodgers <rodgert@twrodgers.com>
Date: Fri, 14 Jan 2022 18:30:27 -0800
Subject: [PATCH] libstdc++: Add missing free functions for atomic_flag
 [PR103934]

libstdc++-v3/ChangeLog:

	PR103934
	* include/std/atomic: Add missing free functions.
	* testsuite/29_atomics/atomic_flag/wait_notify/1.cc:
	Add test case to cover missing atomic_flag free functions.
---
 libstdc++-v3/include/std/atomic               | 39 +++++++++++++++++++
 .../29_atomics/atomic_flag/wait_notify/1.cc   | 27 +++++++++++--
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 9df17704f7e..92c96a9b047 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -1216,6 +1216,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 				    memory_order __m) noexcept
   { return __a->test_and_set(__m); }
 
+#if __cpp_lib_atomic_flag_test
+  inline bool
+  atomic_flag_test(const atomic_flag* __a) noexcept
+  { return __a->test(); }
+
+  inline bool
+  atomic_flag_test(const volatile atomic_flag* __a) noexcept
+  { return __a->test(); }
+
+  inline bool
+  atomic_flag_test_explicit(const atomic_flag* __a,
+			    memory_order __m) noexcept
+  { return __a->test(__m); }
+
+  inline bool
+  atomic_flag_test_explicit(const volatile atomic_flag* __a,
+			    memory_order __m) noexcept
+  { return __a->test(__m); }
+#endif
+
   inline void
   atomic_flag_clear_explicit(atomic_flag* __a, memory_order __m) noexcept
   { __a->clear(__m); }
@@ -1241,6 +1261,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   atomic_flag_clear(volatile atomic_flag* __a) noexcept
   { atomic_flag_clear_explicit(__a, memory_order_seq_cst); }
 
+#if __cpp_lib_atomic_wait
+  inline void
+  atomic_flag_wait(const atomic_flag* __a, bool __old) noexcept
+  { __a->wait(__old); }
+
+  inline void
+  atomic_flag_wait_explicit(const atomic_flag* __a, bool __old,
+		       std::memory_order __m) noexcept
+  { __a->wait(__old, __m); }
+
+  inline void
+  atomic_flag_notify_one(const atomic_flag* __a) noexcept
+  { __a->notify_one(); }
+
+  inline void
+  atomic_flag_notify_all(const atomic_flag* __a) noexcept
+  { __a->notify_all(); }
+#endif // __cpp_lib_atomic_wait
+
 
   template<typename _Tp>
     using __atomic_val_t = typename atomic<_Tp>::value_type;
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc
index 87a104059ff..1050b72a1c6 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc
@@ -26,8 +26,8 @@
 
 #include <testsuite_hooks.h>
 
-int
-main()
+void
+test01()
 {
   std::atomic_flag a;
   VERIFY( !a.test() );
@@ -39,5 +39,26 @@ main()
     });
   a.wait(false);
   t.join();
-  return 0;
+}
+
+void
+test02()
+{
+  std::atomic_flag a;
+  VERIFY( !std::atomic_flag_test(&a) );
+  std::atomic_flag_wait(&a, true);
+  std::thread t([&]
+    {
+      std::atomic_flag_test_and_set(&a);
+      std::atomic_flag_notify_one(&a);
+    });
+  std::atomic_flag_wait(&a, false);
+  t.join();
+}
+
+int
+main()
+{
+  test01();
+  test02();
 }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934]
  2022-01-15  2:53 [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934] Thomas Rodgers
@ 2022-02-02 21:35 ` Jonathan Wakely
  2023-02-10 18:25   ` Thomas Rodgers
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2022-02-02 21:35 UTC (permalink / raw)
  To: Thomas Rodgers; +Cc: libstdc++, gcc Patches

>+  inline void
>+  atomic_flag_wait_explicit(const atomic_flag* __a, bool __old,
>+               std::memory_order __m) noexcept

No need for the std:: qualification, and check the indentation.


> libstdc++-v3/ChangeLog:
>
>    PR103934

This needs to include the component: PR libstdc++/103934

>    * include/std/atomic: Add missing free functions.

Please name the new functions in the changelog, in the usual format.
Just the names is fine, no need for the full signatures with
parameters.

OK for trunk with those changes.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934]
  2022-02-02 21:35 ` Jonathan Wakely
@ 2023-02-10 18:25   ` Thomas Rodgers
  2023-02-11  0:41     ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Rodgers @ 2023-02-10 18:25 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc Patches


[-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --]

This patch did not get committed in a timely manner after it was OK'd. In
revisiting the patch some issues were found that have lead me to resubmit
for review -

Specifically -

The original commit to add C++20 atomic_flag::test did not include the free
functions for atomic_flag_test[_explicit]
The original commit to add C++20 atomic_flag::wait/notify did not include
the free functions for atomic_flag_wait/notify[_explicit]

These two commits landed in GCC10 and GCC11 respectively. My original patch
included both sets of free functions, but
that complicates the backporting of these changes to GCC10, GCC11, and
GCC12.

Additionally commit 7c2155 removed const qualification from
atomic_flag::notify_one/notify_all but the original version of this
patch accepts the atomic flag as const.

The original version of this patch did not include test cases for the
atomic_flag_test[_explicit] free functions.

I have split the original patch into two patches, on for the
atomic_flag_test free functions, and one for the atomic_flag_wait/notify
free functions.


On Wed, Feb 2, 2022 at 1:35 PM Jonathan Wakely <jwakely@redhat.com> wrote:

> >+  inline void
> >+  atomic_flag_wait_explicit(const atomic_flag* __a, bool __old,
> >+               std::memory_order __m) noexcept
>
> No need for the std:: qualification, and check the indentation.
>
>
> > libstdc++-v3/ChangeLog:
> >
> >    PR103934
>
> This needs to include the component: PR libstdc++/103934
>
> >    * include/std/atomic: Add missing free functions.
>
> Please name the new functions in the changelog, in the usual format.
> Just the names is fine, no need for the full signatures with
> parameters.
>
> OK for trunk with those changes.
>
>

[-- Attachment #2: 0002-libstdc-Add-missing-free-functions-for-atomic_flag-P.patch --]
[-- Type: text/x-patch, Size: 2754 bytes --]

From 1338538c1667b7fbe201d22d81254e33a0e7b24c Mon Sep 17 00:00:00 2001
From: Thomas W Rodgers <rodgert@twrodgers.com>
Date: Fri, 10 Feb 2023 10:09:06 -0800
Subject: [PATCH 2/2] libstdc++: Add missing free functions for atomic_flag
 [PR103934]

This patch adds -
  atomic_flag_wait
  atomic_flag_wait_explicit
  atomic_flag_notify
  atomic_flag_notify_explicit

Which were missed when commit 83a1be introduced C++20 atomic wait.

libstdc++-v3/ChangeLog:

	PR libstdc++/103934
	* include/std/atomic: Add missing free functions.
	* testsuite/29_atomics/atomic_flag/wait_notify/1.cc:
	Add test case to cover missing atomic_flag free functions.
---
 libstdc++-v3/include/std/atomic               | 19 ++++++++++++++
 .../29_atomics/atomic_flag/wait_notify/1.cc   | 26 +++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 1edd3ae16fa..e82a6427378 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -1259,6 +1259,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   atomic_flag_clear(volatile atomic_flag* __a) noexcept
   { atomic_flag_clear_explicit(__a, memory_order_seq_cst); }
 
+#if __cpp_lib_atomic_wait
+  inline void
+      atomic_flag_wait(atomic_flag* __a, bool __old) noexcept
+  { __a->wait(__old); }
+
+  inline void
+      atomic_flag_wait_explicit(atomic_flag* __a, bool __old,
+                                memory_order __m) noexcept
+  { __a->wait(__old, __m); }
+
+  inline void
+      atomic_flag_notify_one(atomic_flag* __a) noexcept
+  { __a->notify_one(); }
+
+  inline void
+      atomic_flag_notify_all(atomic_flag* __a) noexcept
+  { __a->notify_all(); }
+#endif // __cpp_lib_atomic_wait
+
   /// @cond undocumented
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
   // 3220. P0558 broke conforming C++14 uses of atomic shared_ptr
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc
index 240fb4259f7..777fa915ea1 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/wait_notify/1.cc
@@ -26,8 +26,8 @@
 
 #include <testsuite_hooks.h>
 
-int
-main()
+void
+test01()
 {
   std::atomic_flag a;
   VERIFY( !a.test() );
@@ -39,5 +39,27 @@ main()
     });
   a.wait(false);
   t.join();
+}
+
+void
+test02()
+{
+  std::atomic_flag a;
+  VERIFY( !std::atomic_flag_test(&a) );
+  std::atomic_flag_wait(&a, true);
+  std::thread t([&]
+    {
+      std::atomic_flag_test_and_set(&a);
+      std::atomic_flag_notify_one(&a);
+    });
+    std::atomic_flag_wait(&a, false);
+    t.join();
+}
+
+int
+main()
+{
+  test01();
+  test02();
   return 0;
 }
-- 
2.39.1


[-- Attachment #3: 0001-libstdc-Add-missing-free-functions-for-atomic_flag-P.patch --]
[-- Type: text/x-patch, Size: 3848 bytes --]

From 7f21e98bbefde5437a116ccda35a85922f5882bf Mon Sep 17 00:00:00 2001
From: Thomas W Rodgers <rodgert@twrodgers.com>
Date: Fri, 10 Feb 2023 09:35:11 -0800
Subject: [PATCH 1/2] libstdc++: Add missing free functions for atomic_flag
 [PR103934]

This patch adds -
  atomic_flag_test
  atomic_flag_test_explicit

Which were missed when commit 491ba6 introduced C++20 atomic flag
test.

libstdc++-v3/ChangeLog:

	PR libstdc++/103934
	* include/std/atomic: Add missing free functions.
	* testsuite/29_atomics/atomic_flag/test/explicit.cc
        Add test case to cover missing atomic_flag free functions.
        * testsuite/29_atomics/atomic_flag/test/explicit.cc
        Likewise.
---
 libstdc++-v3/include/std/atomic               | 20 ++++++++++++++
 .../29_atomics/atomic_flag/test/explicit.cc   | 26 ++++++++++++++++++-
 .../29_atomics/atomic_flag/test/implicit.cc   | 26 ++++++++++++++++++-
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 1c27c8c41e1..1edd3ae16fa 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -1214,6 +1214,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 				    memory_order __m) noexcept
   { return __a->test_and_set(__m); }
 
+#if __cpp_lib_atomic_flag_test
+  inline bool
+  atomic_flag_test(const atomic_flag* __a) noexcept
+  { return __a->test(); }
+
+  inline bool
+  atomic_flag_test(const volatile atomic_flag* __a) noexcept
+  { return __a->test(); }
+
+  inline bool
+  atomic_flag_test_explicit(const atomic_flag* __a,
+			    memory_order __m) noexcept
+  { return __a->test(__m); }
+
+  inline bool
+  atomic_flag_test_explicit(const volatile atomic_flag* __a,
+			    memory_order __m) noexcept
+  { return __a->test(__m); }
+#endif
+
   inline void
   atomic_flag_clear_explicit(atomic_flag* __a, memory_order __m) noexcept
   { __a->clear(__m); }
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/explicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/explicit.cc
index 380bd36ac47..fc8d6589707 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/explicit.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/explicit.cc
@@ -22,7 +22,8 @@
 #include <atomic>
 #include <testsuite_hooks.h>
 
-int main()
+void
+test01()
 {
   using namespace std;
 
@@ -38,3 +39,26 @@ int main()
   VERIFY( ! af.test(memory_order_acquire) );
   VERIFY( ! caf.test(memory_order_acquire) );
 }
+
+void
+test02()
+{
+  using namespace std;
+
+  atomic_flag af{true};
+  const atomic_flag& caf = af;
+
+  VERIFY( atomic_flag_test_explicit(&af, memory_order_acquire) );
+  VERIFY( atomic_flag_test_explicit(&caf, memory_order_acquire) );
+  af.clear(memory_order_release);
+  VERIFY( ! atomic_flag_test_explicit(&af, memory_order_acquire) );
+  VERIFY( ! atomic_flag_test_explicit(&caf, memory_order_acquire) );
+}
+
+int
+main()
+{
+  test01();
+  test02();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/implicit.cc b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/implicit.cc
index cce873fa553..2840055433f 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/implicit.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_flag/test/implicit.cc
@@ -22,7 +22,8 @@
 #include <atomic>
 #include <testsuite_hooks.h>
 
-int main()
+void
+test01()
 {
   using namespace std;
 
@@ -38,3 +39,26 @@ int main()
   VERIFY( ! af.test() );
   VERIFY( ! caf.test() );
 }
+
+void
+test02()
+{
+  using namespace std;
+
+  atomic_flag af{true};
+  const atomic_flag& caf = af;
+
+  VERIFY( atomic_flag_test(&af) );
+  VERIFY( atomic_flag_test(&caf) );
+  af.clear(memory_order_release);
+  VERIFY( ! atomic_flag_test(&af) );
+  VERIFY( ! atomic_flag_test(&caf) );
+}
+
+int
+main()
+{
+  test01();
+  test02();
+  return 0;
+}
-- 
2.39.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934]
  2023-02-10 18:25   ` Thomas Rodgers
@ 2023-02-11  0:41     ` Jonathan Wakely
  2023-02-14  2:06       ` Thomas Rodgers
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2023-02-11  0:41 UTC (permalink / raw)
  To: Thomas Rodgers; +Cc: libstdc++, gcc Patches

On Fri, 10 Feb 2023 at 18:25, Thomas Rodgers <trodgers@redhat.com> wrote:
>
> This patch did not get committed in a timely manner after it was OK'd. In revisiting the patch some issues were found that have lead me to resubmit for review -
>
> Specifically -
>
> The original commit to add C++20 atomic_flag::test did not include the free functions for atomic_flag_test[_explicit]
> The original commit to add C++20 atomic_flag::wait/notify did not include the free functions for atomic_flag_wait/notify[_explicit]
>
> These two commits landed in GCC10 and GCC11 respectively. My original patch included both sets of free functions, but
> that complicates the backporting of these changes to GCC10, GCC11, and GCC12.

I don't think we need them in GCC 10.

> Additionally commit 7c2155 removed const qualification from atomic_flag::notify_one/notify_all but the original version of this
> patch accepts the atomic flag as const.
>
> The original version of this patch did not include test cases for the atomic_flag_test[_explicit] free functions.
>
> I have split the original patch into two patches, on for the atomic_flag_test free functions, and one for the atomic_flag_wait/notify
> free functions.

Thanks.

For [PATCH 1/2] please name the added functions in the changelog entry:

* include/std/atomic (atomic_flag_test): Add.
(atomic_flag_test_explicit): Add.

Similarly for the changelog in [PATCH 2/2], naming the four new
functions added to include/std/atomic.

The indentation is off in [PATCH 2/2] for atomic_flag:

+#if __cpp_lib_atomic_wait
+  inline void
+      atomic_flag_wait(atomic_flag* __a, bool __old) noexcept
+  { __a->wait(__old); }
+

And similarly for the other three added functions.
The function names should start in the same column as the 'inline' and
opening brace of the function body.


Both patches are OK for trunk, gcc-12 and gcc-11 with those changes.




>
>
> On Wed, Feb 2, 2022 at 1:35 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> >+  inline void
>> >+  atomic_flag_wait_explicit(const atomic_flag* __a, bool __old,
>> >+               std::memory_order __m) noexcept
>>
>> No need for the std:: qualification, and check the indentation.
>>
>>
>> > libstdc++-v3/ChangeLog:
>> >
>> >    PR103934
>>
>> This needs to include the component: PR libstdc++/103934
>>
>> >    * include/std/atomic: Add missing free functions.
>>
>> Please name the new functions in the changelog, in the usual format.
>> Just the names is fine, no need for the full signatures with
>> parameters.
>>
>> OK for trunk with those changes.
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934]
  2023-02-11  0:41     ` Jonathan Wakely
@ 2023-02-14  2:06       ` Thomas Rodgers
  2023-03-10  2:39         ` Thomas Rodgers
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Rodgers @ 2023-02-14  2:06 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Tested x86_64-pc-linux-gnu. Pushed to trunk.

The first patch has also been backported and pushed to releases/gcc-12 and
releases/gcc-11

The second patch fails to cleanly cherry-pick. Will resolve and push
shortly.

On Fri, Feb 10, 2023 at 4:41 PM Jonathan Wakely <jwakely@redhat.com> wrote:

> On Fri, 10 Feb 2023 at 18:25, Thomas Rodgers <trodgers@redhat.com> wrote:
> >
> > This patch did not get committed in a timely manner after it was OK'd.
> In revisiting the patch some issues were found that have lead me to
> resubmit for review -
> >
> > Specifically -
> >
> > The original commit to add C++20 atomic_flag::test did not include the
> free functions for atomic_flag_test[_explicit]
> > The original commit to add C++20 atomic_flag::wait/notify did not
> include the free functions for atomic_flag_wait/notify[_explicit]
> >
> > These two commits landed in GCC10 and GCC11 respectively. My original
> patch included both sets of free functions, but
> > that complicates the backporting of these changes to GCC10, GCC11, and
> GCC12.
>
> I don't think we need them in GCC 10.
>
> > Additionally commit 7c2155 removed const qualification from
> atomic_flag::notify_one/notify_all but the original version of this
> > patch accepts the atomic flag as const.
> >
> > The original version of this patch did not include test cases for the
> atomic_flag_test[_explicit] free functions.
> >
> > I have split the original patch into two patches, on for the
> atomic_flag_test free functions, and one for the atomic_flag_wait/notify
> > free functions.
>
> Thanks.
>
> For [PATCH 1/2] please name the added functions in the changelog entry:
>
> * include/std/atomic (atomic_flag_test): Add.
> (atomic_flag_test_explicit): Add.
>
> Similarly for the changelog in [PATCH 2/2], naming the four new
> functions added to include/std/atomic.
>
> The indentation is off in [PATCH 2/2] for atomic_flag:
>
> +#if __cpp_lib_atomic_wait
> +  inline void
> +      atomic_flag_wait(atomic_flag* __a, bool __old) noexcept
> +  { __a->wait(__old); }
> +
>
> And similarly for the other three added functions.
> The function names should start in the same column as the 'inline' and
> opening brace of the function body.
>
>
> Both patches are OK for trunk, gcc-12 and gcc-11 with those changes.
>
>
>
>
> >
> >
> > On Wed, Feb 2, 2022 at 1:35 PM Jonathan Wakely <jwakely@redhat.com>
> wrote:
> >>
> >> >+  inline void
> >> >+  atomic_flag_wait_explicit(const atomic_flag* __a, bool __old,
> >> >+               std::memory_order __m) noexcept
> >>
> >> No need for the std:: qualification, and check the indentation.
> >>
> >>
> >> > libstdc++-v3/ChangeLog:
> >> >
> >> >    PR103934
> >>
> >> This needs to include the component: PR libstdc++/103934
> >>
> >> >    * include/std/atomic: Add missing free functions.
> >>
> >> Please name the new functions in the changelog, in the usual format.
> >> Just the names is fine, no need for the full signatures with
> >> parameters.
> >>
> >> OK for trunk with those changes.
> >>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934]
  2023-02-14  2:06       ` Thomas Rodgers
@ 2023-03-10  2:39         ` Thomas Rodgers
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Rodgers @ 2023-03-10  2:39 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 3396 bytes --]

The second patch has now been backported and pushed to releases/gcc-12 and
releases/gcc-11.

On Mon, Feb 13, 2023 at 6:06 PM Thomas Rodgers <trodgers@redhat.com> wrote:

> Tested x86_64-pc-linux-gnu. Pushed to trunk.
>
> The first patch has also been backported and pushed to releases/gcc-12 and
> releases/gcc-11
>
> The second patch fails to cleanly cherry-pick. Will resolve and push
> shortly.
>
> On Fri, Feb 10, 2023 at 4:41 PM Jonathan Wakely <jwakely@redhat.com>
> wrote:
>
>> On Fri, 10 Feb 2023 at 18:25, Thomas Rodgers <trodgers@redhat.com> wrote:
>> >
>> > This patch did not get committed in a timely manner after it was OK'd.
>> In revisiting the patch some issues were found that have lead me to
>> resubmit for review -
>> >
>> > Specifically -
>> >
>> > The original commit to add C++20 atomic_flag::test did not include the
>> free functions for atomic_flag_test[_explicit]
>> > The original commit to add C++20 atomic_flag::wait/notify did not
>> include the free functions for atomic_flag_wait/notify[_explicit]
>> >
>> > These two commits landed in GCC10 and GCC11 respectively. My original
>> patch included both sets of free functions, but
>> > that complicates the backporting of these changes to GCC10, GCC11, and
>> GCC12.
>>
>> I don't think we need them in GCC 10.
>>
>> > Additionally commit 7c2155 removed const qualification from
>> atomic_flag::notify_one/notify_all but the original version of this
>> > patch accepts the atomic flag as const.
>> >
>> > The original version of this patch did not include test cases for the
>> atomic_flag_test[_explicit] free functions.
>> >
>> > I have split the original patch into two patches, on for the
>> atomic_flag_test free functions, and one for the atomic_flag_wait/notify
>> > free functions.
>>
>> Thanks.
>>
>> For [PATCH 1/2] please name the added functions in the changelog entry:
>>
>> * include/std/atomic (atomic_flag_test): Add.
>> (atomic_flag_test_explicit): Add.
>>
>> Similarly for the changelog in [PATCH 2/2], naming the four new
>> functions added to include/std/atomic.
>>
>> The indentation is off in [PATCH 2/2] for atomic_flag:
>>
>> +#if __cpp_lib_atomic_wait
>> +  inline void
>> +      atomic_flag_wait(atomic_flag* __a, bool __old) noexcept
>> +  { __a->wait(__old); }
>> +
>>
>> And similarly for the other three added functions.
>> The function names should start in the same column as the 'inline' and
>> opening brace of the function body.
>>
>>
>> Both patches are OK for trunk, gcc-12 and gcc-11 with those changes.
>>
>>
>>
>>
>> >
>> >
>> > On Wed, Feb 2, 2022 at 1:35 PM Jonathan Wakely <jwakely@redhat.com>
>> wrote:
>> >>
>> >> >+  inline void
>> >> >+  atomic_flag_wait_explicit(const atomic_flag* __a, bool __old,
>> >> >+               std::memory_order __m) noexcept
>> >>
>> >> No need for the std:: qualification, and check the indentation.
>> >>
>> >>
>> >> > libstdc++-v3/ChangeLog:
>> >> >
>> >> >    PR103934
>> >>
>> >> This needs to include the component: PR libstdc++/103934
>> >>
>> >> >    * include/std/atomic: Add missing free functions.
>> >>
>> >> Please name the new functions in the changelog, in the usual format.
>> >> Just the names is fine, no need for the full signatures with
>> >> parameters.
>> >>
>> >> OK for trunk with those changes.
>> >>
>>
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-10  2:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-15  2:53 [PATCH] libstdc++: Add missing free functions for atomic_flag [PR103934] Thomas Rodgers
2022-02-02 21:35 ` Jonathan Wakely
2023-02-10 18:25   ` Thomas Rodgers
2023-02-11  0:41     ` Jonathan Wakely
2023-02-14  2:06       ` Thomas Rodgers
2023-03-10  2:39         ` Thomas Rodgers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).