From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 1F78638515DA for ; Thu, 2 Sep 2021 17:53:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1F78638515DA Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-558-X70FXSFSPte4RUVnG0vVRg-1; Thu, 02 Sep 2021 13:53:43 -0400 X-MC-Unique: X70FXSFSPte4RUVnG0vVRg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DEDB4518C; Thu, 2 Sep 2021 17:53:42 +0000 (UTC) Received: from localhost (unknown [10.33.36.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8B03810074F1; Thu, 2 Sep 2021 17:53:42 +0000 (UTC) Date: Thu, 2 Sep 2021 18:53:41 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Remove "no stronger" assertion in compare exchange [PR102177] Message-ID: MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="y8rHJNJJIJbCu2L+" Content-Disposition: inline X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Sep 2021 17:53:48 -0000 --y8rHJNJJIJbCu2L+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline P0418R2 removed some preconditions from std::atomic::compare_exchange_* but we still enforce them via __glibcxx_assert. This removes those assertions. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR c++/102177 * include/bits/atomic_base.h (__is_valid_cmpexch_failure_order): New function to check if a memory order is valid for the failure case of compare exchange operations. (__atomic_base::compare_exchange_weak): Simplify assertions by using __is_valid_cmpexch_failure_order. (__atomic_base::compare_exchange_strong): Likewise. (__atomic_base::compare_exchange_weak): Likewise. (__atomic_base::compare_exchange_strong): Likewise. (__atomic_impl::compare_exchange_weak): Add assertion. (__atomic_impl::compare_exchange_strong): Likewise. * include/std/atomic (atomic::compare_exchange_weak): Likewise. (atomic::compare_exchange_strong): Likewise. Tested x86_64-linux. Committed to trunk. I think we should backport this to gcc-11 too, as it was a C++17 change and these assertions fail for valid (if questionable) C++17 programs. --y8rHJNJJIJbCu2L+ Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit dba1ab212292839572fda60df00965e094a11252 Author: Jonathan Wakely Date: Thu Sep 2 15:29:22 2021 libstdc++: Remove "no stronger" assertion in compare exchange [PR102177] P0418R2 removed some preconditions from std::atomic::compare_exchange_* but we still enforce them via __glibcxx_assert. This removes those assertions. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR c++/102177 * include/bits/atomic_base.h (__is_valid_cmpexch_failure_order): New function to check if a memory order is valid for the failure case of compare exchange operations. (__atomic_base::compare_exchange_weak): Simplify assertions by using __is_valid_cmpexch_failure_order. (__atomic_base::compare_exchange_strong): Likewise. (__atomic_base::compare_exchange_weak): Likewise. (__atomic_base::compare_exchange_strong): Likewise. (__atomic_impl::compare_exchange_weak): Add assertion. (__atomic_impl::compare_exchange_strong): Likewise. * include/std/atomic (atomic::compare_exchange_weak): Likewise. (atomic::compare_exchange_strong): Likewise. diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/include/bits/atomic_base.h index 20cf1343c58..cbe1da6d125 100644 --- a/libstdc++-v3/include/bits/atomic_base.h +++ b/libstdc++-v3/include/bits/atomic_base.h @@ -121,6 +121,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION | __memory_order_modifier(__m & __memory_order_modifier_mask)); } + constexpr bool + __is_valid_cmpexch_failure_order(memory_order __m) noexcept + { + return (__m & __memory_order_mask) != memory_order_release + && (__m & __memory_order_mask) != memory_order_acq_rel; + } + _GLIBCXX_ALWAYS_INLINE void atomic_thread_fence(memory_order __m) noexcept { __atomic_thread_fence(int(__m)); } @@ -511,13 +518,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION compare_exchange_weak(__int_type& __i1, __int_type __i2, memory_order __m1, memory_order __m2) noexcept { - memory_order __b2 __attribute__ ((__unused__)) - = __m2 & __memory_order_mask; - memory_order __b1 __attribute__ ((__unused__)) - = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert(__is_valid_cmpexch_failure_order(__m2)); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 1, int(__m1), int(__m2)); @@ -528,13 +529,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __m1, memory_order __m2) volatile noexcept { - memory_order __b2 __attribute__ ((__unused__)) - = __m2 & __memory_order_mask; - memory_order __b1 __attribute__ ((__unused__)) - = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert(__is_valid_cmpexch_failure_order(__m2)); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 1, int(__m1), int(__m2)); @@ -560,13 +555,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION compare_exchange_strong(__int_type& __i1, __int_type __i2, memory_order __m1, memory_order __m2) noexcept { - memory_order __b2 __attribute__ ((__unused__)) - = __m2 & __memory_order_mask; - memory_order __b1 __attribute__ ((__unused__)) - = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert(__is_valid_cmpexch_failure_order(__m2)); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 0, int(__m1), int(__m2)); @@ -577,14 +566,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __m1, memory_order __m2) volatile noexcept { - memory_order __b2 __attribute__ ((__unused__)) - = __m2 & __memory_order_mask; - memory_order __b1 __attribute__ ((__unused__)) - = __m1 & __memory_order_mask; - - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert(__is_valid_cmpexch_failure_order(__m2)); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 0, int(__m1), int(__m2)); @@ -869,13 +851,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __m1, memory_order __m2) noexcept { - memory_order __b2 __attribute__ ((__unused__)) - = __m2 & __memory_order_mask; - memory_order __b1 __attribute__ ((__unused__)) - = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert(__is_valid_cmpexch_failure_order(__m2)); return __atomic_compare_exchange_n(&_M_p, &__p1, __p2, 0, int(__m1), int(__m2)); @@ -886,14 +862,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __m1, memory_order __m2) volatile noexcept { - memory_order __b2 __attribute__ ((__unused__)) - = __m2 & __memory_order_mask; - memory_order __b1 __attribute__ ((__unused__)) - = __m1 & __memory_order_mask; - - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert(__is_valid_cmpexch_failure_order(__m2)); return __atomic_compare_exchange_n(&_M_p, &__p1, __p2, 0, int(__m1), int(__m2)); @@ -996,6 +965,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Val<_Tp> __desired, memory_order __success, memory_order __failure) noexcept { + __glibcxx_assert(__is_valid_cmpexch_failure_order(__failure)); + return __atomic_compare_exchange(__ptr, std::__addressof(__expected), std::__addressof(__desired), true, int(__success), int(__failure)); @@ -1007,6 +978,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Val<_Tp> __desired, memory_order __success, memory_order __failure) noexcept { + __glibcxx_assert(__is_valid_cmpexch_failure_order(__failure)); + return __atomic_compare_exchange(__ptr, std::__addressof(__expected), std::__addressof(__desired), false, int(__success), int(__failure)); diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic index 9b1fb15ac41..b395c65d468 100644 --- a/libstdc++-v3/include/std/atomic +++ b/libstdc++-v3/include/std/atomic @@ -320,6 +320,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION compare_exchange_weak(_Tp& __e, _Tp __i, memory_order __s, memory_order __f) noexcept { + __glibcxx_assert(__is_valid_cmpexch_failure_order(__f)); + return __atomic_compare_exchange(std::__addressof(_M_i), std::__addressof(__e), std::__addressof(__i), @@ -330,6 +332,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION compare_exchange_weak(_Tp& __e, _Tp __i, memory_order __s, memory_order __f) volatile noexcept { + __glibcxx_assert(__is_valid_cmpexch_failure_order(__f)); + return __atomic_compare_exchange(std::__addressof(_M_i), std::__addressof(__e), std::__addressof(__i), @@ -352,6 +356,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION compare_exchange_strong(_Tp& __e, _Tp __i, memory_order __s, memory_order __f) noexcept { + __glibcxx_assert(__is_valid_cmpexch_failure_order(__f)); + return __atomic_compare_exchange(std::__addressof(_M_i), std::__addressof(__e), std::__addressof(__i), @@ -362,6 +368,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION compare_exchange_strong(_Tp& __e, _Tp __i, memory_order __s, memory_order __f) volatile noexcept { + __glibcxx_assert(__is_valid_cmpexch_failure_order(__f)); + return __atomic_compare_exchange(std::__addressof(_M_i), std::__addressof(__e), std::__addressof(__i), --y8rHJNJJIJbCu2L+--