Re: [libstdc++] Inconsistent detection of __int128

Re: [libstdc++] Inconsistent detection of __int128

[Jonathan Wakely]

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

On 30/04/21 16:24 -0400, David Edelsohn via Libstdc++ wrote:
>On Fri, Jan 8, 2021 at 11:10 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> On 03/01/21 22:26 -0500, David Edelsohn via Libstdc++ wrote:
>> >On Sun, Jan 3, 2021 at 9:45 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>> >>
>> >> On Mon, 4 Jan 2021, 00:44 David Edelsohn via Libstdc++, <libstdc++@gcc.gnu.org> wrote:
>> >>>
>> >>> Or is there some other reason that _GLIBCXX_USE_INT128 and
>> >>> __SIZEOF_INT128__ are used in different contexts?
>> >>
>> >> Yes.
>> >>
>> >> I'll reply when I'm back from taking some time off. Probably Wednesday.
>> >
>> >If the uses of _GLIBCXX_USE_INT128 in libstdc++ headers specifically
>> >are checking if __int128 type is different than "long" and "long
>> >long", as opposed to the availability of the __int128, can c++config.h
>>
>> Yes, the test is not just "is __int128 a valid type?" but "should we
>> use __int128 as a larger integer type?"
>>
>> We used to use it more widely, but many places that use __int128
>> conditionally now check __GLIBCXX_INT_N_0 instead, because that is
>> defined when __int128 is available and we're not compiling in a
>> "strict ansi" dialect.
>>
>> Of the two remaining uses of _GLIBCXX_USE_INT128 I think one is wrong
>> and should test __SIZEOF__INT128__ directly. We specifically do want
>> to use unsigned __int128 in __to_chars_unsigned_type even if it's the
>> same as a standard type.
>>
>> So that leaves one remaining use, and I think that should do as you
>> suggest here ...
>>
>> >define _GLIBCXX_USE_INT128 by comparing __SIZEOF_INT128__ to
>> >__SIZEOF_LONG_LONG__ and __SIZEOF_LONG__ instead of the libstdc++
>> >configure template typename test?
>>
>> Yes, that seems fine. But since we only need it in one place, let's
>> just do that test in that one place. It makes the purpose of the test
>> more explicit, and so you don't need to look up what
>> _GLIBCXX_USE_INT128 means and when it's true.
>>
>> Does the attached patch work for you?
>
>Hi, Jonathan
>
>Thanks for pushing the INT64_T patch.
>
>I tested your revised patch in January and it works perfectly.
>
>Could you push your INT128 patch to trunk when you have a moment?
>
>Thanks, David

I've pushed the attached to trunk after testing on powerpc-aix and
x86_64-linux.

Contrary to what I said in January, I think both sues of
_GLIBCXX_USE_INT128 do want to only use __int128 if it's larger than
long long. For __to_chars_unsigned_type there's no point trying to use
unsigned __int128 if it's the same size as unsigned long long, because
we'd already have chosen unsigned long long.




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

commit ad0a3be4df5eecc79075d899fd79179d0f61270e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 12:07:09 2021

    libstdc++: Remove _GLIBCXX_USE_INT128 autoconf macro
    
    We don't need to decide whether to use __int128 when running configure,
    we can do so at compilation time by seeing if __SIZEOF_INT128__ is
    defined and if it's greater than __SIZEOF_LONG_LONG__.
    
    This removes another unnecessary architecture-specific config macro in
    <bits/c++config.h>, so the same header can work for 32-bit or 64-bit
    compilation on AIX.
    
    libstdc++-v3/ChangeLog:
    
            * acinclude.m4 (GLIBCXX_ENABLE_INT128_FLOAT128): Remove
            checks for __int128 and rename to GLIBCXX_ENABLE_FLOAT128.
            * config.h.in: Regenerate.
            * configure: Regenerate.
            * configure.ac: Adjust to use GLIBCXX_ENABLE_FLOAT128.
            * include/bits/random.h (_Select_uint_least_t<s, 1>):
            Use __SIZEOF_INT128__ to decide whether to use __int128.
            * include/std/charconv (__to_chars_unsigned_type): Likewise.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 7b78e148fbd..94897a654c9 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3049,15 +3049,14 @@ EOF
 ])
 
 dnl
-dnl Check for GNU 128-bit integer and floating point types.
+dnl Check for GNU 128-bit floating point type.
 dnl
-dnl Note: also checks that the types aren't standard types.
+dnl Note: also checks that the type isn't a standard types.
 dnl
 dnl Defines:
-dnl  _GLIBCXX_USE_INT128
 dnl  ENABLE_FLOAT128
 dnl
-AC_DEFUN([GLIBCXX_ENABLE_INT128_FLOAT128], [
+AC_DEFUN([GLIBCXX_ENABLE_FLOAT128], [
 
   AC_LANG_SAVE
   AC_LANG_CPLUSPLUS
@@ -3065,34 +3064,7 @@ AC_DEFUN([GLIBCXX_ENABLE_INT128_FLOAT128], [
   # Fake what AC_TRY_COMPILE does, without linking as this is
   # unnecessary for this test.
 
-    cat > conftest.$ac_ext << EOF
-[#]line __oline__ "configure"
-template<typename T1, typename T2>
-  struct same
-  { typedef T2 type; };
-
-template<typename T>
-  struct same<T, T>;
-
-int main()
-{
-  typename same<long, __int128>::type                i1;
-  typename same<long long, __int128>::type           i2;
-}
-EOF
-
-    AC_MSG_CHECKING([for __int128])
-    if AC_TRY_EVAL(ac_compile); then
-      AC_DEFINE(_GLIBCXX_USE_INT128, 1,
-      [Define if __int128 is supported on this host.])
-      enable_int128=yes
-    else
-      enable_int128=no
-    fi
-    AC_MSG_RESULT($enable_int128)
-    rm -f conftest*
-
-    cat > conftest.$ac_ext << EOF
+  cat > conftest.$ac_ext << EOF
 [#]line __oline__ "configure"
 template<typename T1, typename T2>
   struct same
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 95dd9ce5da5..a816ff79d16 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -153,7 +153,7 @@ GLIBCXX_ENABLE_THREADS
 GLIBCXX_ENABLE_ATOMIC_BUILTINS
 GLIBCXX_ENABLE_LOCK_POLICY
 GLIBCXX_ENABLE_DECIMAL_FLOAT
-GLIBCXX_ENABLE_INT128_FLOAT128
+GLIBCXX_ENABLE_FLOAT128
 if test "$enable_float128" = yes; then
   port_specific_symbol_files="$port_specific_symbol_files \$(top_srcdir)/config/abi/pre/float128.ver"
 fi
diff --git a/libstdc++-v3/include/bits/random.h b/libstdc++-v3/include/bits/random.h
index 1b9cc5f16a9..0da013c5f45 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -99,7 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       struct _Select_uint_least_t<__s, 2>
       { typedef unsigned long long type; };
 
-#ifdef _GLIBCXX_USE_INT128
+#if __SIZEOF_INT128__ > __SIZEOF_LONG_LONG__
     template<int __s>
       struct _Select_uint_least_t<__s, 1>
       { typedef unsigned __int128 type; };
diff --git a/libstdc++-v3/include/std/charconv b/libstdc++-v3/include/std/charconv
index 6e407f31e30..193702e677a 100644
--- a/libstdc++-v3/include/std/charconv
+++ b/libstdc++-v3/include/std/charconv
@@ -94,7 +94,7 @@ namespace __detail
     struct __to_chars_unsigned_type : __make_unsigned_selector_base
     {
       using _UInts = _List<unsigned int, unsigned long, unsigned long long
-#if _GLIBCXX_USE_INT128
+#if __SIZEOF_INT128__ > __SIZEOF_LONG_LONG__
 	, unsigned __int128
 #endif
 	>;

Re: [libstdc++] Inconsistent detection of __int128

[David Edelsohn]

On Tue, May 4, 2021 at 7:12 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 30/04/21 16:24 -0400, David Edelsohn via Libstdc++ wrote:
> >On Fri, Jan 8, 2021 at 11:10 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >> On 03/01/21 22:26 -0500, David Edelsohn via Libstdc++ wrote:
> >> >On Sun, Jan 3, 2021 at 9:45 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >> >>
> >> >> On Mon, 4 Jan 2021, 00:44 David Edelsohn via Libstdc++, <libstdc++@gcc.gnu.org> wrote:
> >> >>>
> >> >>> Or is there some other reason that _GLIBCXX_USE_INT128 and
> >> >>> __SIZEOF_INT128__ are used in different contexts?
> >> >>
> >> >> Yes.
> >> >>
> >> >> I'll reply when I'm back from taking some time off. Probably Wednesday.
> >> >
> >> >If the uses of _GLIBCXX_USE_INT128 in libstdc++ headers specifically
> >> >are checking if __int128 type is different than "long" and "long
> >> >long", as opposed to the availability of the __int128, can c++config.h
> >>
> >> Yes, the test is not just "is __int128 a valid type?" but "should we
> >> use __int128 as a larger integer type?"
> >>
> >> We used to use it more widely, but many places that use __int128
> >> conditionally now check __GLIBCXX_INT_N_0 instead, because that is
> >> defined when __int128 is available and we're not compiling in a
> >> "strict ansi" dialect.
> >>
> >> Of the two remaining uses of _GLIBCXX_USE_INT128 I think one is wrong
> >> and should test __SIZEOF__INT128__ directly. We specifically do want
> >> to use unsigned __int128 in __to_chars_unsigned_type even if it's the
> >> same as a standard type.
> >>
> >> So that leaves one remaining use, and I think that should do as you
> >> suggest here ...
> >>
> >> >define _GLIBCXX_USE_INT128 by comparing __SIZEOF_INT128__ to
> >> >__SIZEOF_LONG_LONG__ and __SIZEOF_LONG__ instead of the libstdc++
> >> >configure template typename test?
> >>
> >> Yes, that seems fine. But since we only need it in one place, let's
> >> just do that test in that one place. It makes the purpose of the test
> >> more explicit, and so you don't need to look up what
> >> _GLIBCXX_USE_INT128 means and when it's true.
> >>
> >> Does the attached patch work for you?
> >
> >Hi, Jonathan
> >
> >Thanks for pushing the INT64_T patch.
> >
> >I tested your revised patch in January and it works perfectly.
> >
> >Could you push your INT128 patch to trunk when you have a moment?
> >
> >Thanks, David
>
> I've pushed the attached to trunk after testing on powerpc-aix and
> x86_64-linux.
>
> Contrary to what I said in January, I think both sues of
> _GLIBCXX_USE_INT128 do want to only use __int128 if it's larger than
> long long. For __to_chars_unsigned_type there's no point trying to use
> unsigned __int128 if it's the same size as unsigned long long, because
> we'd already have chosen unsigned long long.

Hi, Jonathan

Excellent.  Thank you very much for addressing this issue and removing
the multilib size dependencies in libstdc++ headers.

Thanks, David