On Wed, 1 Dec 2021 at 18:24, Jonathan Wakely wrote:
>
> On Wed, 1 Dec 2021 at 18:16, Florian Weimer wrote:
> >
> > * Jonathan Wakely via Libstdc:
> >
> > > diff --git a/libstdc++-v3/include/bits/cow_string.h
> > > b/libstdc++-v3/include/bits/cow_string.h
> > > index ced395b80b8..4fae1d02981 100644
> > > --- a/libstdc++-v3/include/bits/cow_string.h
> > > +++ b/libstdc++-v3/include/bits/cow_string.h
> > > @@ -105,7 +105,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > * destroy the empty-string _Rep object.
> > > *
> > > * All but the last paragraph is considered pretty conventional
> > > - * for a C++ string implementation.
> > > + * for a Copy-On-Write C++ string implementation.
> > > */
> > > // 21.3 Template class basic_string
> > > template<typename _CharT, typename _Traits, typename _Alloc>
> > > @@ -207,10 +207,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > // so we need to use an atomic load. However, _M_is_leaked
> > > // predicate does not change concurrently (i.e. the string is
> > > either
> > > // leaked or not), so a relaxed load is enough.
> > > - return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
> > > -#else
> > > - return this->_M_refcount < 0;
> > > + if (!__gnu_cxx::__is_single_threaded())
> > > + return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) <
> > > 0;
> > > #endif
> > > + return this->_M_refcount < 0;
> > > }
> >
> > Relaxed MO loads of word-size values on all current architectures only
> > have a compiler barrier, so I think the optimization makes things worse?
>
> Hmm, yes.
>
> > (I doubt the conditional lack of a compiler barrier leads to
> > optimization improvements elsewhere.)
>
> Probably not. I'll revert the change to _M_is_leaked() and just keep
> it for _M_is_shared().
>
> Thanks for pointing that out.
Reverted by the attached patch, tested powerpc64le-linux.
commit b5a568683f71b4a8b1e4e45a43484398e9a66ff2
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Wed Dec 1 20:58:58 2021
libstdc++: Restore unconditional atomic load in COW std::string
The relaxed load is already optimal, checking the __single_threaded
global before doing a non-atomic load isn't an optimization.
libstdc++-v3/ChangeLog:
* include/bits/cow_string.h (basic_string::_M_is_leaked()):
Revert change to check __is_single_threaded() before using
atomic load.
diff --git a/libstdc++-v3/include/bits/cow_string.h
b/libstdc++-v3/include/bits/cow_string.h
index d6ddf3489d1..389b39583e4 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -207,10 +207,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// so we need to use an atomic load. However, _M_is_leaked
// predicate does not change concurrently (i.e. the string is either
// leaked or not), so a relaxed load is enough.
- if (!__gnu_cxx::__is_single_threaded())
- return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
-#endif
+ return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
+#else
return this->_M_refcount < 0;
+#endif
}
bool
