Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2023-01-12 Thread Jonathan Wakely via Gcc-patches 
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++ 
>  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  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.

Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-28 Thread François Dumont via Gcc-patches 

On 28/11/22 19:35, Jonathan Wakely wrote:



On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ 
mailto:libstdc%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  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
diff --git a/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver b/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
index 06ccaa80a58..7fc81514808 100644
--- a/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
+++ b/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
@@ -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:
 *;
 };
diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 23ffbdabed8..fb7a02cec57 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -1288,15 +1288,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("_ZNSt3__88to_charsEPcS0_DF128_");
+#  else
 __asm("_ZSt8to_charsPcS_DF128_");
+#  endif
 
   to_chars_result
   to_chars(char*, char*, _Float128, chars_format) noexcept
+#  if _GLIBCXX_INLINE_VERSION
+__asm("_ZNSt3__88to_charsEPcS0_DF128_NS_12chars_formatE");
+#  else
 __asm("_ZSt8to_charsPcS_DF128_St12chars_format");
+#  endif
 
   to_chars_result
   to_chars(char*, char*, _Float128, chars_format, int) noexcept
+#  if _GLIBCXX_INLINE_VERSION
+__asm("_ZNSt3__88to_charsEPcS0_DF128_NS_12chars_formatEi");
+#  else
 __asm("_ZSt8to_charsPcS_DF128_St12chars_formati");
+#  endif
 # endif
 #endif

Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-28 Thread François Dumont via Gcc-patches 
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  wrote:



On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++
mailto:libstdc%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  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  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

Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-28 Thread François Dumont via Gcc-patches 

On 28/11/22 11:21, Jonathan Wakely wrote:



On Mon, 28 Nov 2022 at 10:10, Jonathan Wakely  wrote:



On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++
mailto:libstdc%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  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  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

Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-28 Thread Jonathan Wakely via Gcc-patches 
On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <
libstd...@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  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.

Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-28 Thread Jonathan Wakely via Gcc-patches 
On Mon, 28 Nov 2022 at 10:10, Jonathan Wakely  wrote:

>
>
> On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <
> libstd...@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  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

Re: [PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-28 Thread Jonathan Wakely via Gcc-patches 
On Mon, 28 Nov 2022 at 06:07, François Dumont via Libstdc++ <
libstd...@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  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::*;
};

[PATCH][_GLIBCXX_INLINE_VERSION] Adapt to_chars/from_chars symbols

2022-11-27 Thread François Dumont via Gcc-patches 

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  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
diff --git a/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver b/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
index 06ccaa80a58..7fc81514808 100644
--- a/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
+++ b/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
@@ -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:
 *;
 };
diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 23ffbdabed8..a05f3981622 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -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