From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 64B8B3857402 for ; Fri, 30 Apr 2021 19:31:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 64B8B3857402 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-196-nPJb5QtRPE-Ez5M9fKmGSg-1; Fri, 30 Apr 2021 15:31:33 -0400 X-MC-Unique: nPJb5QtRPE-Ez5M9fKmGSg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AAD1610066E6; Fri, 30 Apr 2021 19:31:31 +0000 (UTC) Received: from localhost (unknown [10.33.36.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4E8DF1043297; Fri, 30 Apr 2021 19:31:30 +0000 (UTC) Date: Fri, 30 Apr 2021 20:31:29 +0100 From: Jonathan Wakely To: David Edelsohn Cc: Jakub Jelinek , Iain Sandoe , libstdc++ , GCC Patches , "CHIGOT, CLEMENT" Subject: Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T Message-ID: <20210430193129.GC3008@redhat.com> References: <20210106193746.GD725145@tucnak> <20210106214159.GF725145@tucnak> <20210106233937.GH725145@tucnak> <20210106234549.GA3748@tucnak> <20210108183703.GF9471@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="pWAGJYLEitsbNOt2" Content-Disposition: inline X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Apr 2021 19:31:39 -0000 --pWAGJYLEitsbNOt2 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On 29/04/21 16:06 -0400, David Edelsohn wrote: >On Fri, Jan 8, 2021 at 1:37 PM Jonathan Wakely wrote: >> >> On 06/01/21 19:41 -0500, David Edelsohn wrote: >> >Thanks for clarifying the issue. >> > >> >As you implicitly point out, GCC knows the type of INT64 and defines >> >the macro __INT64_TYPE__ . The revised code can use that directly, >> >such as: >> > >> >#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ >> > || defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) >> > typedef __INT64_TYPE__ streamoff; >> > #elif defined(_GLIBCXX_HAVE_INT64_T) >> > typedef int64_t streamoff; >> > #else >> > typedef long long streamoff; >> > #endif >> > >> >Are there any additional issues not addressed by that approach, other >> >than possible further simplification? >> >> That avoids the ABI break that Jakub pointed out. But I think we can >> simplify it further, as in the attached patch. >> >> This uses __INT64_TYPE__ if that's defined, and long long otherwise. I >> think that should be equivalent in all practical cases (I can imagine >> some strange target where __INT64_TYPE__ is defined by the compiler, >> but int64_t isn't defined when the configure checks look for it, and >> so the current code would use long long and with my patch would use >> __INT64_TYPE__ which could be long ... but I think in practice that's >> unlikely. It was probably more likely in older releases where the >> configure test would have been done with -std=gnu++98 and so int64_t >> might not have been declared by libc's , but if that was the >> case then any ABI break it caused happened years ago. > >Hi, Jonathan > >Polite ping. > >Now that GCC 11.1 has been released, can this patch be applied to >libstdc++? As I replied at the time to Jakub's concerns, both Clang >(since 3.0.0) and ICC (since at least 16.0.0) have defined >__INT64_TYPE__ . Pushed to trunk after testing on x86_64-linux and powerpc-aix. --pWAGJYLEitsbNOt2 Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit 7ddcd26ebb60a8a4b57330442631115cabb6ec22 Author: Jonathan Wakely Date: Fri Apr 30 15:54:14 2021 libstdc++: Remove GLIBCXX_CHECK_INT64_T checks This simplifies the definition of std::streamoff by using the predefined __INT64_TYPE__ macro, instead of the _GLIBCXX_HAVE_INT64_T_LONG, _GLIBCXX_HAVE_INT64_T_LONG_LONG and _GLIBCXX_HAVE_INT64_T macros defined by configure. By using the __INT64_TYPE__ macro (which all of GCC, Clang and Intel define) we do not need to determine the type of int64_t in configure, we can just use that type directly. The background for the change was explained by David Edelsohn: Currently the type of streamoff is determined at libstdc++ configure time, chosen by the definitions of _GLIBCXX_HAVE_INT64_T_LONG and _GLIBCXX_HAVE_INT64_T_LONG_LONG. For a multilib configuration, the difference is encoded in the different multilib header file paths. For "FAT" library targets that package 32 bit and 64 bit libraries together, G++ also expects a single header file directory hierarchy, causing an incorrect value for streamoff in some situations. And in a subsequent mail: Most of the libstdc++ headers are architecture-neutral, OS neutral and ABI neutral. The differences are localized in bits/c++config.h. And most of c++config.h is identical for 32 bit AIX and 64 bit AIX. The only differences that matter are __int128 and __int64_t. This change removes some of those differences. With the only uses of the INT64_T configure macros removed, the configure checks themselves can also be removed. Co-authored-by: David Edelsohn libstdc++-v3/ChangeLog: * acinclude.m4 (GLIBCXX_CHECK_INT64_T): Delete. * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Do not use GLIBCXX_CHECK_INT64_T. * include/bits/postypes.h: Remove include of and definition/undefinition of the __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS macros. (streamoff): Use __INT64_TYPE__ if defined. diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index 1c0a4c13052..7b78e148fbd 100644 --- a/libstdc++-v3/acinclude.m4 +++ b/libstdc++-v3/acinclude.m4 @@ -474,63 +474,6 @@ AC_DEFUN([GLIBCXX_CHECK_WRITEV], [ ]) -dnl -dnl Check whether int64_t is available in , and define HAVE_INT64_T. -dnl Also check whether int64_t is actually a typedef to long or long long. -dnl -AC_DEFUN([GLIBCXX_CHECK_INT64_T], [ - - AC_LANG_SAVE - AC_LANG_CPLUSPLUS - - AC_MSG_CHECKING([for int64_t]) - AC_CACHE_VAL(glibcxx_cv_INT64_T, [ - AC_TRY_COMPILE( - [#include ], - [int64_t var;], - [glibcxx_cv_INT64_T=yes], - [glibcxx_cv_INT64_T=no]) - ]) - - if test $glibcxx_cv_INT64_T = yes; then - AC_DEFINE(HAVE_INT64_T, 1, [Define if int64_t is available in .]) - AC_MSG_RESULT($glibcxx_cv_INT64_T) - - AC_MSG_CHECKING([for int64_t as long]) - AC_CACHE_VAL(glibcxx_cv_int64_t_long, [ - AC_TRY_COMPILE( - [#include - template struct same { enum { value = -1 }; }; - template struct same { enum { value = 1 }; }; - int array[same::value];], [], - [glibcxx_cv_int64_t_long=yes], [glibcxx_cv_int64_t_long=no]) - ]) - - if test $glibcxx_cv_int64_t_long = yes; then - AC_DEFINE(HAVE_INT64_T_LONG, 1, [Define if int64_t is a long.]) - AC_MSG_RESULT($glibcxx_cv_int64_t_long) - fi - - AC_MSG_CHECKING([for int64_t as long long]) - AC_CACHE_VAL(glibcxx_cv_int64_t_long_long, [ - AC_TRY_COMPILE( - [#include - template struct same { enum { value = -1 }; }; - template struct same { enum { value = 1 }; }; - int array[same::value];], [], - [glibcxx_cv_int64_t_long_long=yes], [glibcxx_cv_int64_t_long_long=no]) - ]) - - if test $glibcxx_cv_int64_t_long_long = yes; then - AC_DEFINE(HAVE_INT64_T_LONG_LONG, 1, [Define if int64_t is a long long.]) - AC_MSG_RESULT($glibcxx_cv_int64_t_long_long) - fi - fi - - AC_LANG_RESTORE -]) - - dnl dnl Check whether LFS support is available. dnl diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 3c799be82b1..95dd9ce5da5 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -185,9 +185,6 @@ GLIBCXX_CHECK_STDIO_PROTO GLIBCXX_CHECK_MATH11_PROTO GLIBCXX_CHECK_UCHAR_H -# For the streamoff typedef. -GLIBCXX_CHECK_INT64_T - # For LFS support. GLIBCXX_CHECK_LFS diff --git a/libstdc++-v3/include/bits/postypes.h b/libstdc++-v3/include/bits/postypes.h index cb44cfe1396..52590ddd61b 100644 --- a/libstdc++-v3/include/bits/postypes.h +++ b/libstdc++-v3/include/bits/postypes.h @@ -39,32 +39,6 @@ #include // For mbstate_t -// XXX If is really needed, make sure to define the macros -// before including it, in order not to break (and -// in C++11). Reconsider all this as soon as possible... -#if (defined(_GLIBCXX_HAVE_INT64_T) && !defined(_GLIBCXX_HAVE_INT64_T_LONG) \ - && !defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG)) - -#ifndef __STDC_LIMIT_MACROS -# define _UNDEF__STDC_LIMIT_MACROS -# define __STDC_LIMIT_MACROS -#endif -#ifndef __STDC_CONSTANT_MACROS -# define _UNDEF__STDC_CONSTANT_MACROS -# define __STDC_CONSTANT_MACROS -#endif -#include // For int64_t -#ifdef _UNDEF__STDC_LIMIT_MACROS -# undef __STDC_LIMIT_MACROS -# undef _UNDEF__STDC_LIMIT_MACROS -#endif -#ifdef _UNDEF__STDC_CONSTANT_MACROS -# undef __STDC_CONSTANT_MACROS -# undef _UNDEF__STDC_CONSTANT_MACROS -#endif - -#endif - namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION @@ -84,12 +58,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * Note: In versions of GCC up to and including GCC 3.3, streamoff * was typedef long. */ -#ifdef _GLIBCXX_HAVE_INT64_T_LONG - typedef long streamoff; -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) - typedef long long streamoff; -#elif defined(_GLIBCXX_HAVE_INT64_T) - typedef int64_t streamoff; +#ifdef __INT64_TYPE__ + typedef __INT64_TYPE__ streamoff; #else typedef long long streamoff; #endif --pWAGJYLEitsbNOt2--