[_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols
Checks
Commit Message
This patch is fixing those tests:
20_util/to_chars/float128_c++23.cc
std/format/formatter/requirements.cc
std/format/functions/format.cc
std/format/functions/format_to_n.cc
std/format/functions/size.cc
std/format/functions/vformat_to.cc
std/format/string.cc
Note that symbols used in <format> for __ibm128 and __iee128 are untested.
I even wonder if the normal mode ones are because I cannot find the
symbols used in gnu.ver.
libstdc++: [_GLIBCXX_INLINE_VERSION] Add to_chars/from_chars
symbols export
libstdc++-v3/ChangeLog
* include/std/format [_GLIBCXX_INLINE_VERSION](to_chars):
Adapt __asm symbol
specifications.
* config/abi/pre/gnu-versioned-namespace.ver: Add
to_chars/from_chars symbols
export.
Ok to commit ?
François
Comments
On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:
> This patch is fixing those tests:
>
> 20_util/to_chars/float128_c++23.cc
> std/format/formatter/requirements.cc
> std/format/functions/format.cc
> std/format/functions/format_to_n.cc
> std/format/functions/size.cc
> std/format/functions/vformat_to.cc
> std/format/string.cc
>
> Note that symbols used in <format> for __ibm128 and __iee128 are untested.
>
> I even wonder if the normal mode ones are because I cannot find the
> symbols used in gnu.ver.
>
>
> libstdc++: [_GLIBCXX_INLINE_VERSION] Add to_chars/from_chars
> symbols export
>
> libstdc++-v3/ChangeLog
>
> * include/std/format [_GLIBCXX_INLINE_VERSION](to_chars):
> Adapt __asm symbol
> specifications.
> * config/abi/pre/gnu-versioned-namespace.ver: Add
> to_chars/from_chars symbols
> export.
>
> Ok to commit ?
>
>
Why are changes needed to the linker script?
Those functions should already match the general wildcard:
# Names inside the 'extern' block are demangled names.
extern "C++"
{
std::*;
std::__8::*;
};
On Mon, 28 Nov 2022 at 10:10, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>
> On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <
> libstdc++@gcc.gnu.org> wrote:
>
>> This patch is fixing those tests:
>>
>> 20_util/to_chars/float128_c++23.cc
>> std/format/formatter/requirements.cc
>> std/format/functions/format.cc
>> std/format/functions/format_to_n.cc
>> std/format/functions/size.cc
>> std/format/functions/vformat_to.cc
>> std/format/string.cc
>>
>> Note that symbols used in <format> for __ibm128 and __iee128 are untested.
>>
>> I even wonder if the normal mode ones are because I cannot find the
>> symbols used in gnu.ver.
>>
>>
>> libstdc++: [_GLIBCXX_INLINE_VERSION] Add to_chars/from_chars
>> symbols export
>>
>> libstdc++-v3/ChangeLog
>>
>> * include/std/format [_GLIBCXX_INLINE_VERSION](to_chars):
>> Adapt __asm symbol
>> specifications.
>> * config/abi/pre/gnu-versioned-namespace.ver: Add
>> to_chars/from_chars symbols
>> export.
>>
>> Ok to commit ?
>>
>>
>
> Why are changes needed to the linker script?
>
> Those functions should already match the general wildcard:
>
> # Names inside the 'extern' block are demangled names.
> extern "C++"
> {
> std::*;
> std::__8::*;
> };
>
>
>
Instead of nine separate #if blocks, can we just do:
#if _GLIBCXX_INLINE_VERSION
# define _GLIBCXX_ALIAS(S) __asm("_ZNSt3__8" S)
#else
# define _GLIBCXX_ALIAS(S) __asm("_ZNSt" S)
#endif
And then use:
_GLIBCXX_ALIAS("8to_charsPcS_eSt12chars_format");
and finally:
#undef _GLIBCXX_ALIAS
On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:
> This patch is fixing those tests:
>
> 20_util/to_chars/float128_c++23.cc
> std/format/formatter/requirements.cc
> std/format/functions/format.cc
> std/format/functions/format_to_n.cc
> std/format/functions/size.cc
> std/format/functions/vformat_to.cc
> std/format/string.cc
>
> Note that symbols used in <format> for __ibm128 and __iee128 are untested.
>
We don't need to do this for those symbols, the ALT128 config is
incompatible with versioned namespace. If you're using the versioned
namespace, you don't need backwards compatibility with the old long double
ABI.
On 28/11/22 11:21, Jonathan Wakely wrote:
>
>
> On Mon, 28 Nov 2022 at 10:10, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>
>
> On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
>
> This patch is fixing those tests:
>
> 20_util/to_chars/float128_c++23.cc
> std/format/formatter/requirements.cc
> std/format/functions/format.cc
> std/format/functions/format_to_n.cc
> std/format/functions/size.cc
> std/format/functions/vformat_to.cc
> std/format/string.cc
>
> Note that symbols used in <format> for __ibm128 and __iee128
> are untested.
>
> I even wonder if the normal mode ones are because I cannot
> find the
> symbols used in gnu.ver.
>
>
> libstdc++: [_GLIBCXX_INLINE_VERSION] Add to_chars/from_chars
> symbols export
>
> libstdc++-v3/ChangeLog
>
> * include/std/format
> [_GLIBCXX_INLINE_VERSION](to_chars):
> Adapt __asm symbol
> specifications.
> * config/abi/pre/gnu-versioned-namespace.ver: Add
> to_chars/from_chars symbols
> export.
>
> Ok to commit ?
>
>
>
> Why are changes needed to the linker script?
>
> Those functions should already match the general wildcard:
>
> # Names inside the 'extern' block are demangled names.
> extern "C++"
> {
> std::*;
> std::__8::*;
> };
>
>
>
No idear, my guess was that it has something to do with the __asm usages
in <format> and with the commnt:
// These overloads exist in the library, but are not declared for C++20.
// Make them available as std::__format::to_chars.
Maybe they exist in the library but are unused so not exported unless
specified in the linker script ?
>
> Instead of nine separate #if blocks, can we just do:
>
> #if _GLIBCXX_INLINE_VERSION
> # define _GLIBCXX_ALIAS(S) __asm("_ZNSt3__8" S)
> #else
> # define _GLIBCXX_ALIAS(S) __asm("_ZNSt" S)
> #endif
>
> And then use:
>
> _GLIBCXX_ALIAS("8to_charsPcS_eSt12chars_format");
>
> and finally:
>
> #undef _GLIBCXX_ALIAS
>
>
I tried and as expected it's not working because the diff in the symbol
is not limited to the '3__8' pattern. 'chars_format' is also defined in
versioned namespace which might perhaps explain some mangling diff.
Here is an updated patch though, I had forgotten to replace a _DF128
with a __ieee128 in the untested part of this patch.
If you prefer to take a closer look later I'll just re-submit my patch
to move versioned namespace mode to cxx11 abi knowing that those tests
are already FAIL.
François
I forgot to add the patch but as you already made another feedback I'll
clean my patch first.
On 28/11/22 19:43, François Dumont wrote:
> On 28/11/22 11:21, Jonathan Wakely wrote:
>>
>>
>> On Mon, 28 Nov 2022 at 10:10, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>>
>>
>> On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++
>> <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
>>
>> This patch is fixing those tests:
>>
>> 20_util/to_chars/float128_c++23.cc
>> std/format/formatter/requirements.cc
>> std/format/functions/format.cc
>> std/format/functions/format_to_n.cc
>> std/format/functions/size.cc
>> std/format/functions/vformat_to.cc
>> std/format/string.cc
>>
>> Note that symbols used in <format> for __ibm128 and __iee128
>> are untested.
>>
>> I even wonder if the normal mode ones are because I cannot
>> find the
>> symbols used in gnu.ver.
>>
>>
>> libstdc++: [_GLIBCXX_INLINE_VERSION] Add
>> to_chars/from_chars
>> symbols export
>>
>> libstdc++-v3/ChangeLog
>>
>> * include/std/format
>> [_GLIBCXX_INLINE_VERSION](to_chars):
>> Adapt __asm symbol
>> specifications.
>> * config/abi/pre/gnu-versioned-namespace.ver: Add
>> to_chars/from_chars symbols
>> export.
>>
>> Ok to commit ?
>>
>>
>>
>> Why are changes needed to the linker script?
>>
>> Those functions should already match the general wildcard:
>>
>> # Names inside the 'extern' block are demangled names.
>> extern "C++"
>> {
>> std::*;
>> std::__8::*;
>> };
>>
>>
>>
> No idear, my guess was that it has something to do with the __asm
> usages in <format> and with the commnt:
>
> // These overloads exist in the library, but are not declared for C++20.
> // Make them available as std::__format::to_chars.
>
> Maybe they exist in the library but are unused so not exported unless
> specified in the linker script ?
>
>
>>
>> Instead of nine separate #if blocks, can we just do:
>>
>> #if _GLIBCXX_INLINE_VERSION
>> # define _GLIBCXX_ALIAS(S) __asm("_ZNSt3__8" S)
>> #else
>> # define _GLIBCXX_ALIAS(S) __asm("_ZNSt" S)
>> #endif
>>
>> And then use:
>>
>> _GLIBCXX_ALIAS("8to_charsPcS_eSt12chars_format");
>>
>> and finally:
>>
>> #undef _GLIBCXX_ALIAS
>>
>>
> I tried and as expected it's not working because the diff in the
> symbol is not limited to the '3__8' pattern. 'chars_format' is also
> defined in versioned namespace which might perhaps explain some
> mangling diff.
>
> Here is an updated patch though, I had forgotten to replace a _DF128
> with a __ieee128 in the untested part of this patch.
>
> If you prefer to take a closer look later I'll just re-submit my patch
> to move versioned namespace mode to cxx11 abi knowing that those tests
> are already FAIL.
>
> François
>
>
On 28/11/22 19:35, Jonathan Wakely wrote:
>
>
> On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
>
> This patch is fixing those tests:
>
> 20_util/to_chars/float128_c++23.cc
> std/format/formatter/requirements.cc
> std/format/functions/format.cc
> std/format/functions/format_to_n.cc
> std/format/functions/size.cc
> std/format/functions/vformat_to.cc
> std/format/string.cc
>
> Note that symbols used in <format> for __ibm128 and __iee128 are
> untested.
>
>
> We don't need to do this for those symbols, the ALT128 config is
> incompatible with versioned namespace. If you're using the versioned
> namespace, you don't need backwards compatibility with the old long
> double ABI.
>
>
>
Here is the simplified patch then.
libstdc++: [_GLIBCXX_INLINE_VERSION] Add to_chars/from_chars
symbols export
libstdc++-v3/ChangeLog
* include/std/format [_GLIBCXX_INLINE_VERSION](to_chars):
Adapt __asm symbol
specifications.
* config/abi/pre/gnu-versioned-namespace.ver: Add
to_chars/from_chars symbols
export.
Ok to commit ?
François
On Mon, 28 Nov 2022 at 20:44, François Dumont wrote:
>
> On 28/11/22 19:35, Jonathan Wakely wrote:
>
>
>
> On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>
>> This patch is fixing those tests:
>>
>> 20_util/to_chars/float128_c++23.cc
>> std/format/formatter/requirements.cc
>> std/format/functions/format.cc
>> std/format/functions/format_to_n.cc
>> std/format/functions/size.cc
>> std/format/functions/vformat_to.cc
>> std/format/string.cc
>>
>> Note that symbols used in <format> for __ibm128 and __iee128 are untested.
>
>
> We don't need to do this for those symbols, the ALT128 config is incompatible with versioned namespace. If you're using the versioned namespace, you don't need backwards compatibility with the old long double ABI.
>
>
>
>
> Here is the simplified patch then.
>
> libstdc++: [_GLIBCXX_INLINE_VERSION] Add to_chars/from_chars symbols export
>
> libstdc++-v3/ChangeLog
>
> * include/std/format [_GLIBCXX_INLINE_VERSION](to_chars): Adapt __asm symbol
> specifications.
> * config/abi/pre/gnu-versioned-namespace.ver: Add to_chars/from_chars symbols
> export.
>
> Ok to commit ?
I still don't understand why the linker script change is needed, but I
can investigate that myself later.
OK for trunk, thanks.
@@ -142,6 +142,12 @@ GLIBCXX_8.0 {
_ZN14__gnu_parallel9_Settings3getEv;
_ZN14__gnu_parallel9_Settings3setERS0_;
+ # to_chars/from_chars _Float128
+ _ZNSt3__88to_charsEPcS0_DF128_;
+ _ZNSt3__88to_charsEPcS0_DF128_NS_12chars_formatE;
+ _ZNSt3__88to_charsEPcS0_DF128_NS_12chars_formatEi;
+ _ZNSt3__810from_charsEPKcS1_RDF128_NS_12chars_formatE;
+
local:
*;
};
@@ -1248,27 +1248,51 @@ namespace __format
// Make them available as std::__format::to_chars.
to_chars_result
to_chars(char*, char*, __ibm128) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_e");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_e");
+# endif
to_chars_result
to_chars(char*, char*, __ibm128, chars_format) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_eSt12chars_format");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_eNS_12chars_formatE");
+# endif
to_chars_result
to_chars(char*, char*, __ibm128, chars_format, int) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_eSt12chars_formati");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_eNS_12chars_formatEi");
+# endif
#elif __cplusplus == 202002L
to_chars_result
to_chars(char*, char*, __ieee128) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_u9__ieee128");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_DF128_");
+# endif
to_chars_result
to_chars(char*, char*, __ieee128, chars_format) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_u9__ieee128St12chars_format");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_u9__ieee128NS_12chars_formatE");
+# endif
to_chars_result
to_chars(char*, char*, __ieee128, chars_format, int) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_u9__ieee128St12chars_formati");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_u9__ieee128NS_12chars_formatEi");
+# endif
#endif
#elif defined _GLIBCXX_LDOUBLE_IS_IEEE_BINARY128
@@ -1288,15 +1312,27 @@ namespace __format
// Make them available as std::__format::to_chars.
to_chars_result
to_chars(char*, char*, _Float128) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_DF128_");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_DF128_");
+# endif
to_chars_result
to_chars(char*, char*, _Float128, chars_format) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_DF128_St12chars_format");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_DF128_NS_12chars_formatE");
+# endif
to_chars_result
to_chars(char*, char*, _Float128, chars_format, int) noexcept
+# if !_GLIBCXX_INLINE_VERSION
__asm("_ZSt8to_charsPcS_DF128_St12chars_formati");
+# else
+ __asm("_ZNSt3__88to_charsEPcS0_DF128_NS_12chars_formatEi");
+# endif
# endif
#endif