From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp002.apm-internet.net (smtp002.apm-internet.net [85.119.248.221]) by sourceware.org (Postfix) with ESMTPS id E87D93858D1E for ; Mon, 9 Oct 2023 16:27:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E87D93858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sandoe.co.uk Received: (qmail 48146 invoked from network); 9 Oct 2023 16:27:14 -0000 X-APM-Out-ID: 16968688344813 X-APM-Authkey: 257869/1(257869/1) 11 Received: from unknown (HELO smtpclient.apple) (81.138.1.83) by smtp002.apm-internet.net with SMTP; 9 Oct 2023 16:27:14 -0000 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.4\)) Subject: Re: [PATCH] sso-string@gnu-versioned-namespace [PR83077] From: Iain Sandoe In-Reply-To: Date: Mon, 9 Oct 2023 17:27:14 +0100 Cc: =?utf-8?Q?Fran=C3=A7ois_Dumont?= , Jonathan Wakely , libstdc++ , GCC Patches Content-Transfer-Encoding: quoted-printable Message-Id: <0F6C3451-18B4-418A-A42F-63054BDA48CF@sandoe.co.uk> References: <1dc681f4-41b7-d171-02ac-b0194617bdee@gmail.com> <91dc6383-6bff-ce6c-b24d-81cd2ab2dce8@gmail.com> <5306fc1f-603b-43ef-beec-420cbe0ebfca@gmail.com> To: Jonathan Wakely X-Mailer: Apple Mail (2.3696.120.41.1.4) X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_COUK,KAM_DMARC_STATUS,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > On 9 Oct 2023, at 15:42, Iain Sandoe wrote: >> On 7 Oct 2023, at 20:32, Fran=C3=A7ois Dumont = wrote: >>=20 >> I've been told that previous patch generated with 'git diff -b' was = not applying properly so here is the same patch again with a simple 'git = diff'. >=20 > Thanks, that did fix it - There are some training whitespaces in the = config files, but I suspect that they need to be there since those have = values appended during the configuration. >=20 > Anyway, with this + the coroutines and contract v2 (weak def) fix, = plus a local patch to enable versioned namespace on Darwin, I get = results comparable with the non-versioned case - but one more patchlet = is needed on yours (to allow for targets using emultated TLS): >=20 > diff --git a/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver = b/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver > index 9fab8bead15..b7167fc0c2f 100644 > --- a/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver > +++ b/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver > @@ -78,6 +78,7 @@ GLIBCXX_8.0 { >=20 > # thread/mutex/condition_variable/future > __once_proxy; > + __emutls_v._ZNSt3__81?__once_call*; >=20 > # std::__convert_to_v > _ZNSt3__814__convert_to_v*; Having said this, since the versioned lib is an ABI-break, perhaps we = should also take the opportunity to fix the once_call impl. here too? (at least the fix I made locally does not need the TLS var, so ths would = then be moot) Iain >=20 > thanks > Iain >=20 >>=20 >>=20 >> On 07/10/2023 14:25, Fran=C3=A7ois Dumont wrote: >>> Hi >>>=20 >>> Here is a rebased version of this patch. >>>=20 >>> There are few test failures when running 'make check-c++' but = nothing new. >>>=20 >>> Still, there are 2 patches awaiting validation to fix some of them, = PR c++/111524 to fix another bunch and I fear that we will have to live = with the others. >>>=20 >>> libstdc++: [_GLIBCXX_INLINE_VERSION] Use cxx11 abi [PR83077] >>>=20 >>> Use cxx11 abi when activating versioned namespace mode. To do = support >>> a new configuration mode where !_GLIBCXX_USE_DUAL_ABI and = _GLIBCXX_USE_CXX11_ABI. >>>=20 >>> The main change is that std::__cow_string is now defined whenever = _GLIBCXX_USE_DUAL_ABI >>> or _GLIBCXX_USE_CXX11_ABI is true. Implementation is using = available std::string in >>> case of dual abi and a subset of it when it's not. >>>=20 >>> On the other side std::__sso_string is defined only when = _GLIBCXX_USE_DUAL_ABI is true >>> and _GLIBCXX_USE_CXX11_ABI is false. Meaning that = std::__sso_string is a typedef for the >>> cow std::string implementation when dual abi is disabled and cow = string is being used. >>>=20 >>> libstdcxx-v3/ChangeLog: >>>=20 >>> PR libstdc++/83077 >>> * acinclude.m4 [GLIBCXX_ENABLE_LIBSTDCXX_DUAL_ABI]: = Default to "new" libstdcxx abi >>> when enable_symvers is gnu-versioned-namespace. >>> * config/locale/dragonfly/monetary_members.cc = [!_GLIBCXX_USE_DUAL_ABI]: Define money_base >>> members. >>> * config/locale/generic/monetary_members.cc = [!_GLIBCXX_USE_DUAL_ABI]: Likewise. >>> * config/locale/gnu/monetary_members.cc = [!_GLIBCXX_USE_DUAL_ABI]: Likewise. >>> * config/locale/gnu/numeric_members.cc >>> [!_GLIBCXX_USE_DUAL_ABI](__narrow_multibyte_chars): = Define. >>> * configure: Regenerate. >>> * include/bits/c++config >>> [_GLIBCXX_INLINE_VERSION](_GLIBCXX_NAMESPACE_CXX11, = _GLIBCXX_BEGIN_NAMESPACE_CXX11): >>> Define empty. >>> [_GLIBCXX_INLINE_VERSION](_GLIBCXX_END_NAMESPACE_CXX11, = _GLIBCXX_DEFAULT_ABI_TAG): >>> Likewise. >>> * include/bits/cow_string.h [!_GLIBCXX_USE_CXX11_ABI]: = Define a light version of COW >>> basic_string as __std_cow_string for use in stdexcept. >>> * include/std/stdexcept [_GLIBCXX_USE_CXX11_ABI]: Define = __cow_string. >>> (__cow_string(const char*)): New. >>> (__cow_string::c_str()): New. >>> * python/libstdcxx/v6/printers.py = (StdStringPrinter::__init__): Set self.new_string to True >>> when std::__8::basic_string type is found. >>> * src/Makefile.am = [ENABLE_SYMVERS_GNU_NAMESPACE](ldbl_alt128_compat_sources): Define = empty. >>> * src/Makefile.in: Regenerate. >>> * src/c++11/Makefile.am (cxx11_abi_sources): Rename = into... >>> (dual_abi_sources): ...this. Also move cow-local_init.cc, = cxx11-hash_tr1.cc, >>> cxx11-ios_failure.cc entries to... >>> (sources): ...this. >>> (extra_string_inst_sources): Move cow-fstream-inst.cc, = cow-sstream-inst.cc, cow-string-inst.cc, >>> cow-string-io-inst.cc, cow-wtring-inst.cc, = cow-wstring-io-inst.cc, cxx11-locale-inst.cc, >>> cxx11-wlocale-inst.cc entries to... >>> (inst_sources): ...this. >>> * src/c++11/Makefile.in: Regenerate. >>> * src/c++11/cow-fstream-inst.cc [_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cow-locale_init.cc [_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cow-sstream-inst.cc [_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cow-stdexcept.cc [_GLIBCXX_USE_CXX11_ABI]: = Include . >>> [_GLIBCXX_USE_DUAL_ABI || = _GLIBCXX_USE_CXX11_ABI](__cow_string): Redefine before >>> including . Define = _GLIBCXX_DEFINE_STDEXCEPT_INSTANTIATIONS so that >>> __cow_string definition in is skipped. >>> [_GLIBCXX_USE_CXX11_ABI]: Skip Transaction Memory TS = definitions. >>> * src/c++11/string-inst.cc: Add sizeof/alignof = static_assert on stdexcept >>> __cow_string definition. >>> (_GLIBCXX_DEFINING_CXX11_ABI_INSTANTIATIONS): Define = following _GLIBCXX_USE_CXX11_ABI >>> value. >>> [_GLIBCXX_USE_CXX11_ABI && = !_GLIBCXX_DEFINING_CXX11_ABI_INSTANTIATIONS]: >>> Define _GLIBCXX_DEFINING_COW_STRING_INSTANTIATIONS. = Include . >>> Define basic_string as __std_cow_string for the current = translation unit. >>> * src/c++11/cow-string-inst.cc [_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cow-string-io-inst.cc = [_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>> * src/c++11/cow-wstring-inst.cc [_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cow-wstring-io-inst.cc = [_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>> * src/c++11/cxx11-hash_tr1.cc [!_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cxx11-ios_failure.cc = [!_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>> [!_GLIBCXX_USE_DUAL_ABI] (__ios_failure): Remove. >>> * src/c++11/cxx11-locale-inst.cc: Cleanup, just include = locale-inst.cc. >>> * src/c++11/cxx11-stdexcept.cc [!_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++11/cxx11-wlocale-inst.cc = [!_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>> * src/c++11/locale-inst-numeric.h >>> [!_GLIBCXX_USE_DUAL_ABI](std::use_facet>, = std::use_facet>): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](std::has_facet>, = std::has_facet>): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](std::num_get>): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](std::num_put>): Instantiate. >>> * src/c++11/locale-inst.cc [!_GLIBCXX_USE_DUAL_ABI]: = Build only when configured >>> _GLIBCXX_USE_CXX11_ABI is equal to currently built abi. >>> [!_GLIBCXX_USE_DUAL_ABI](__moneypunct_cache): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__moneypunct_cache): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__numpunct_cache): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__timepunct): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__timepunct_cache): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](time_put>): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](time_put_byname>): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__ctype_abstract_base): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](ctype_byname): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__codecvt_abstract_base): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](codecvt_byname): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet>(const locale&)): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet>(const locale&)): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet<__timepunct>(const locale&)): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet>(const locale&)): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet>(const locale&)): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet>(const locale&)): Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet<__timepunct>(const locale&)): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet>(const locale&)): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__add_grouping): Define. >>> [!_GLIBCXX_USE_DUAL_ABI](__pad>): = Instantiate. >>> [!_GLIBCXX_USE_DUAL_ABI](__int_to_char(C*, unsigned long, = const C*, >>> ios_base::fmtflags, bool)): Define. >>> [!_GLIBCXX_USE_DUAL_ABI](__int_to_char(C*, unsigned long = long, const C*, >>> ios_base::fmtflags, bool)): Define. >>> * src/c++11/wlocale-inst.cc [!_GLIBCXX_USE_CXX11_ABI]: = Skip definitions. >>> * src/c++98/Makefile.am (cxx11_abi_sources): Remove, = unique cow-istream-string.cc entry >>> move to... >>> (inst_sources): ...this. >>> * src/c++98/Makefile.in: Regenerate. >>> * src/c++98/cow-istream-string.cc: Include = . >>> [_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>> * src/c++98/hash_tr1.cc [_GLIBCXX_USE_CXX11_ABI]: Skip = definitions. >>> * src/c++98/ios_failure.cc = [_GLIBCXX_USE_CXX11_ABI][_GLIBCXX_USE_DUAL_ABI]: Skip definitions. >>> * src/c++98/istream-string.cc [!_GLIBCXX_USE_DUAL_ABI]: = Build only when configured >>> _GLIBCXX_USE_CXX11_ABI is equal to currently built abi. >>> * src/c++98/locale_facets.cc = [_GLIBCXX_USE_CXX11_ABI](__verify_grouping): Remove. >>> * src/c++98/stdexcept.cc >>> [_GLIBCXX_USE_CXX11_ABI](logic_error(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](domain_error(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](invalid_argument(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](length_error(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](out_of_range(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](runtime_error(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](range_error(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](overflow_error(const string&): = Remove. >>> [_GLIBCXX_USE_CXX11_ABI](underflow_error(const string&): = Remove. >>> * src/c++98/compatibility.cc [_GLIBCXX_USE_CXX11_ABI]: = Skip all definitions appart from >>> istream::ignore(streamsize). >>>=20 >>> Tested under Linux x64_86, ok to commit ? >>>=20 >>> Fran=C3=A7ois >>>=20 >>>=20 >>> On 17/08/2023 19:22, Jonathan Wakely wrote: >>>> On Sun, 13 Aug 2023 at 14:27, Fran=C3=A7ois Dumont via Libstdc++ >>>> wrote: >>>>> Here is the fixed patch tested in all 3 modes: >>>>>=20 >>>>> - _GLIBCXX_USE_DUAL_ABI >>>>>=20 >>>>> - !_GLIBCXX_USE_DUAL_ABI && !_GLIBCXX_USE_CXX11_ABI >>>>>=20 >>>>> - !_GLIBCXX_USE_DUAL_ABI && _GLIBCXX_USE_CXX11_ABI >>>>>=20 >>>>> I don't know what you have in mind for the change below but I = wanted to >>>>> let you know that I tried to put COW std::basic_string into a = nested >>>>> __cow namespace when _GLIBCXX_USE_CXX11_ABI. But it had more = impact on >>>>> string-inst.cc so I preferred the macro substitution approach. >>>> I was thinking of implementing the necessary special members = functions >>>> of __cow_string directly, so they are ABI compatible with the COW >>>> std::basic_string but don't actually reuse the code. That would = mean >>>> we don't need to compile and instantiate the whole COW string just = to >>>> use a few members from it. But that can be done later, the macro >>>> approach seems OK for now. >>>>=20 >>>>> There are some test failing when !_GLIBCXX_USE_CXX11_ABI that are >>>>> unrelated with my changes. I'll propose fixes in coming days. >>>> Which tests? I run the entire testsuite with >>>> -D_GLIBCXX_USE_CXX11_ABI=3D0 several times per day and I'm not = seeing >>>> failures. >>>>=20 >>>> I'll review the patch ASAP, thanks for working on it. >> >=20