[committed] libstdc++: Do not use volatile for __gnu_cxx::rope reference counting

[committed] libstdc++: Do not use volatile for __gnu_cxx::rope reference counting

[Jonathan Wakely]

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

The rope extension uses a volatile variable for its reference count.
This is not only unnecessary for correctness (volatile provides neither
atomicity nor memory visibility, and the variable is only modified while
a lock is held) but it now causes deprecated warnings with
-Wsystem-headers due to the use of ++ and -- operators.

It would be possible to use __gnu_cxx::__exchange_and_add in _M_incr and
_M_decr when __atomic_is_lock_free(sizeof(_RC_t), &_M_ref_count) is
true, rather than locking a mutex. That would probably be a significant
improvement for multi-threaded and single-threaded code (because
__exchange_and_add will use non-atomic ops when possible, and even in MT
code it should be faster than the mutex lock/unlock pair). However,
mixing objects compiled with the old and new code would result in
inconsistent synchronization being used for the reference count.

libstdc++-v3/ChangeLog:

	* include/ext/rope (_Refcount_Base::_M_ref_count): Remove
	volatile qualifier.
	(_Refcount_Base::_M_decr()): Likewise.

Tested x86_64-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2043 bytes --]

commit d067bd72936aaaa7e947e919fc869143539ae023
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 29 14:47:17 2020

    libstdc++: Do not use volatile for __gnu_cxx::rope reference counting
    
    The rope extension uses a volatile variable for its reference count.
    This is not only unnecessary for correctness (volatile provides neither
    atomicity nor memory visibility, and the variable is only modified while
    a lock is held) but it now causes deprecated warnings with
    -Wsystem-headers due to the use of ++ and -- operators.
    
    It would be possible to use __gnu_cxx::__exchange_and_add in _M_incr and
    _M_decr when __atomic_is_lock_free(sizeof(_RC_t), &_M_ref_count) is
    true, rather than locking a mutex. That would probably be a significant
    improvement for multi-threaded and single-threaded code (because
    __exchange_and_add will use non-atomic ops when possible, and even in MT
    code it should be faster than the mutex lock/unlock pair). However,
    mixing objects compiled with the old and new code would result in
    inconsistent synchronization being used for the reference count.
    
    libstdc++-v3/ChangeLog:
    
            * include/ext/rope (_Refcount_Base::_M_ref_count): Remove
            volatile qualifier.
            (_Refcount_Base::_M_decr()): Likewise.

diff --git a/libstdc++-v3/include/ext/rope b/libstdc++-v3/include/ext/rope
index fb7bdb0d6f4..08e510bb0dc 100644
--- a/libstdc++-v3/include/ext/rope
+++ b/libstdc++-v3/include/ext/rope
@@ -452,7 +452,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     typedef std::size_t _RC_t;
     
     // The data member _M_ref_count
-    volatile _RC_t _M_ref_count;
+    _RC_t _M_ref_count;
 
     // Constructor
 #ifdef __GTHREAD_MUTEX_INIT
@@ -489,7 +489,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _M_decr()
     {
       __gthread_mutex_lock(&_M_ref_count_lock);
-      volatile _RC_t __tmp = --_M_ref_count;
+      _RC_t __tmp = --_M_ref_count;
       __gthread_mutex_unlock(&_M_ref_count_lock);
       return __tmp;
     }