* [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T @ 2021-01-06 18:38 David Edelsohn 2021-01-06 18:55 ` Marc Glisse 2021-01-06 19:37 ` Jakub Jelinek 0 siblings, 2 replies; 17+ messages in thread From: David Edelsohn @ 2021-01-06 18:38 UTC (permalink / raw) To: Jonathan Wakely; +Cc: GCC Patches, libstdc++, Iain Sandoe, CHIGOT, CLEMENT Use __SIZEOF_LONG__ and __SIZEOF_LONG_LONG__ to determine the type of streamoff at compile time instead of _GLIBCXX_HAVE_INT64_T_LONG and _GLIBCXX_HAVE_INT64_T_LONG_LONG. 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. This patch changes the only use of _GLIBCXX_HAVE_INT64_T_XXX to test __SIZEOF_LONG__ and __SIZEOF__LONG_LONG__ at compile time. Because libstdc++ explicitly probes the type of __int64_t to choose the declaration of streamoff, this patch only performs the dynamic determination if either _GLIBCXX_HAVE_INT64_T_LONG or _GLIBCXX_HAVE_INT64_T_LONG_LONG are defined. In other words, it does assume that if either one is defined, the other is defined, i.e., that __int64_t is a typedef for one of those two types when one or the other is present. But it then dynamically chooses the type based on the size of "long" and "long long" instead of using the configure time value so that the header dynamically adapts to the 32/64 mode of GCC. If neither __GLIBCXX_HAVE_INT64_T_XXX macro is defined, the original logic proceeds, using either __int64_t or "long long". Based on the configure time definitions, the size should be one or the other when one of the macros is defined. I can add an error case if neither __SIZEOF_LONG__ nor __SIZEOF_LONG_LONG__ are 8 despite __GLIBCXX_HAVE_INT64_T_XXX defined. Is this an acceptable solution to determine the value at compile-time? Thanks, David diff --git a/libstdc++-v3/include/bits/postypes.h b/libstdc++-v3/include/bits/postypes.h index cb44cfe1396..81b9c4c6ae5 100644 --- a/libstdc++-v3/include/bits/postypes.h +++ b/libstdc++-v3/include/bits/postypes.h @@ -84,10 +84,15 @@ _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 +#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ + || defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) + +#if __SIZEOF_LONG__ == 8 typedef long streamoff; -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) +#elif __SIZEOF_LONG_LONG__ == 8 typedef long long streamoff; +#endif + #elif defined(_GLIBCXX_HAVE_INT64_T) typedef int64_t streamoff; #else ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 18:38 [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T David Edelsohn @ 2021-01-06 18:55 ` Marc Glisse 2021-01-06 19:11 ` David Edelsohn 2021-01-06 19:37 ` Jakub Jelinek 1 sibling, 1 reply; 17+ messages in thread From: Marc Glisse @ 2021-01-06 18:55 UTC (permalink / raw) To: David Edelsohn Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Wed, 6 Jan 2021, David Edelsohn via Gcc-patches wrote: > 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. Shouldn't we change that? I don't see why using the same directory for linking should imply using the same directory for includes. -- Marc Glisse ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 18:55 ` Marc Glisse @ 2021-01-06 19:11 ` David Edelsohn 0 siblings, 0 replies; 17+ messages in thread From: David Edelsohn @ 2021-01-06 19:11 UTC (permalink / raw) To: libstdc++; +Cc: Jonathan Wakely, Iain Sandoe, GCC Patches, CHIGOT, CLEMENT On Wed, Jan 6, 2021 at 1:55 PM Marc Glisse <marc.glisse@inria.fr> wrote: > > On Wed, 6 Jan 2021, David Edelsohn via Gcc-patches wrote: > > > 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. > > Shouldn't we change that? I don't see why using the same directory for > linking should imply using the same directory for includes. First, the search path assumption is rather strongly embedded in the GCC driver in somewhat delicate code. There already is a lot of fragile, complicated processing for multilibs and search paths and exception. It is more risky, both in the potential new search logic and in possible breakage, to perform surgery on the multilib support code. Second, it's confusing and error-prone to have different search behavior for different parts of the compiler until we can completely remove multilib headers. Third, there is no inherent reason that the header files should be multilib-dependent. I'm not trying to boil the ocean and remove all multilib differences in headers. I'm trying to solve specific issues caused by the differences in header files that also happen to move GCC in a direction of less multilib differences encoded in header files, which I think is a GOOD THING[tm]. Thanks, David ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 18:38 [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T David Edelsohn 2021-01-06 18:55 ` Marc Glisse @ 2021-01-06 19:37 ` Jakub Jelinek 2021-01-06 21:20 ` David Edelsohn 1 sibling, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2021-01-06 19:37 UTC (permalink / raw) To: David Edelsohn Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Wed, Jan 06, 2021 at 01:38:25PM -0500, David Edelsohn via Gcc-patches wrote: > Is this an acceptable solution to determine the value at compile-time? This looks wrong to me. The fact that long is 64-bit doesn't imply that int64_t as defined by stdint.h must be long, it could be long long too. And while e.g. for C it doesn't really matter much whether streamoff will be long or long long if those two have the same precision, for C++ it matters a lot (affects mangling etc.). > diff --git a/libstdc++-v3/include/bits/postypes.h > b/libstdc++-v3/include/bits/postypes.h > index cb44cfe1396..81b9c4c6ae5 100644 > --- a/libstdc++-v3/include/bits/postypes.h > +++ b/libstdc++-v3/include/bits/postypes.h > @@ -84,10 +84,15 @@ _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 > +#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ > + || defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > + > +#if __SIZEOF_LONG__ == 8 > typedef long streamoff; > -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > +#elif __SIZEOF_LONG_LONG__ == 8 > typedef long long streamoff; > +#endif > + > #elif defined(_GLIBCXX_HAVE_INT64_T) > typedef int64_t streamoff; > #else Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 19:37 ` Jakub Jelinek @ 2021-01-06 21:20 ` David Edelsohn 2021-01-06 21:41 ` Jakub Jelinek 0 siblings, 1 reply; 17+ messages in thread From: David Edelsohn @ 2021-01-06 21:20 UTC (permalink / raw) To: Jakub Jelinek Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Wed, Jan 6, 2021 at 2:37 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Jan 06, 2021 at 01:38:25PM -0500, David Edelsohn via Gcc-patches wrote: > > Is this an acceptable solution to determine the value at compile-time? > > This looks wrong to me. The fact that long is 64-bit doesn't imply that > int64_t as defined by stdint.h must be long, it could be long long too. > And while e.g. for C it doesn't really matter much whether streamoff > will be long or long long if those two have the same precision, > for C++ it matters a lot (affects mangling etc.). Your response doesn't correspond to the patch nor to what I described. Nowhere did I say that int64_t must correspond to "long". The patch specifically chooses "long" or "long long" based on the __SIZEOF_LONG__ *and* __SIZEOF_LONG_LONG__. Currently libstdc++ configure tests for the type at configuration time. My patch changes the behavior to retain the test for the type at configure time but chooses "long" or "long long" at compile time. I don't unilaterally choose "long" or "long long" as the type, but rely on the configure test to ensure that __int64_t is a typedef for either "long" or "long long". The patch does assume that if __int64_t is a typedef for "long" or "long long" and this is a 32/64 multilib, then the typedef for the other 32/64 mode is an equivalent typedef, which is the case for GLIBC, AIX, and other systems that I have checked. Thanks, David > > > diff --git a/libstdc++-v3/include/bits/postypes.h > > b/libstdc++-v3/include/bits/postypes.h > > index cb44cfe1396..81b9c4c6ae5 100644 > > --- a/libstdc++-v3/include/bits/postypes.h > > +++ b/libstdc++-v3/include/bits/postypes.h > > @@ -84,10 +84,15 @@ _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 > > +#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ > > + || defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > > + > > +#if __SIZEOF_LONG__ == 8 > > typedef long streamoff; > > -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > > +#elif __SIZEOF_LONG_LONG__ == 8 > > typedef long long streamoff; > > +#endif > > + > > #elif defined(_GLIBCXX_HAVE_INT64_T) > > typedef int64_t streamoff; > > #else > > Jakub > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 21:20 ` David Edelsohn @ 2021-01-06 21:41 ` Jakub Jelinek 2021-01-06 23:01 ` David Edelsohn 0 siblings, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2021-01-06 21:41 UTC (permalink / raw) To: David Edelsohn Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Wed, Jan 06, 2021 at 04:20:22PM -0500, David Edelsohn wrote: > Your response doesn't correspond to the patch nor to what I described. > > Nowhere did I say that int64_t must correspond to "long". The patch > specifically chooses "long" or "long long" based on the > __SIZEOF_LONG__ *and* __SIZEOF_LONG_LONG__. > > Currently libstdc++ configure tests for the type at configuration > time. My patch changes the behavior to retain the test for the type > at configure time but chooses "long" or "long long" at compile time. > I don't unilaterally choose "long" or "long long" as the type, but > rely on the configure test to ensure that __int64_t is a typedef for > either "long" or "long long". > > The patch does assume that if __int64_t is a typedef for "long" or > "long long" and this is a 32/64 multilib, then the typedef for the > other 32/64 mode is an equivalent typedef, which is the case for > GLIBC, AIX, and other systems that I have checked. Yes, on glibc it is the case, but it doesn't have to be the case on other OSes, which is why there is a configure check. Other OSes can typedef int64_t to whatever they want (or what somebody choose years ago and is now an ABI issue). So, if you wanted to do this, you'd need to check at configure time both multilibs and determine what to do for both. I don't really understand the problem you are trying to solve, because normally the c++config.h header that defines _GLIBCXX_HAVE_INT64_T_LONG etc. comes from a multilib specific header directory, e.g. on powerpc64-linux, one has: /usr/include/c++/11/powerpc64-unknown-linux-gnu/bits/c++config.h and /usr/include/c++/11/powerpc64-unknown-linux-gnu/32/bits/c++config.h (or instead /64/, depending on which multilib is the default) and g++ driver arranges for the search paths to first look at the multilib specific subdirectory and only later at the generic one. Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 21:41 ` Jakub Jelinek @ 2021-01-06 23:01 ` David Edelsohn 2021-01-06 23:39 ` Jakub Jelinek 0 siblings, 1 reply; 17+ messages in thread From: David Edelsohn @ 2021-01-06 23:01 UTC (permalink / raw) To: Jakub Jelinek Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Wed, Jan 6, 2021 at 4:42 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Jan 06, 2021 at 04:20:22PM -0500, David Edelsohn wrote: > > Your response doesn't correspond to the patch nor to what I described. > > > > Nowhere did I say that int64_t must correspond to "long". The patch > > specifically chooses "long" or "long long" based on the > > __SIZEOF_LONG__ *and* __SIZEOF_LONG_LONG__. > > > > Currently libstdc++ configure tests for the type at configuration > > time. My patch changes the behavior to retain the test for the type > > at configure time but chooses "long" or "long long" at compile time. > > I don't unilaterally choose "long" or "long long" as the type, but > > rely on the configure test to ensure that __int64_t is a typedef for > > either "long" or "long long". > > > > The patch does assume that if __int64_t is a typedef for "long" or > > "long long" and this is a 32/64 multilib, then the typedef for the > > other 32/64 mode is an equivalent typedef, which is the case for > > GLIBC, AIX, and other systems that I have checked. > > Yes, on glibc it is the case, but it doesn't have to be the case on other > OSes, which is why there is a configure check. Other OSes can typedef > int64_t to whatever they want (or what somebody choose years ago and is now > an ABI issue). > So, if you wanted to do this, you'd need to check at configure time both > multilibs and determine what to do for both. You continue to not respond to the actual patch and to raise issues that don't exist in the actual patch. libstdc++ configure tests if __int64_t has any type, and separately tests if __int64_t uses typedef "long" or "long long", setting separate macros. The patch uses __SIZEOF_LONG__ and __SIZEOF_LONG_LONG__ if _GLIBCXX_HAVE_INT64_T_LONG or _GLIBCXX_HAVE_INT64_T_LONG_LONG is defined -- exactly for the situation where configure already has determined that __int64_t is either "long" or "long long" and not some other definition or type. If those are not defined, the patch falls back to the current, existing behavior which uses __int64_t, if __int64_t is defined, or "long long" if nothing is defined. This is exactly how the current code behaves if __int64_t is not "long" nor "long long". So, exactly as you state, __int64_t could be a different typedef and the patch continues to use that typedef if the OS didn'f use "long" or "long long". What is not clear about that and how does that not address your concern? > > I don't really understand the problem you are trying to solve, because > normally the c++config.h header that defines _GLIBCXX_HAVE_INT64_T_LONG etc. > comes from a multilib specific header directory, e.g. on powerpc64-linux, > one has: > /usr/include/c++/11/powerpc64-unknown-linux-gnu/bits/c++config.h > and > /usr/include/c++/11/powerpc64-unknown-linux-gnu/32/bits/c++config.h > (or instead /64/, depending on which multilib is the default) and > g++ driver arranges for the search paths to first look at the multilib > specific subdirectory and only later at the generic one. AIX uses what Darwin calls "FAT" libraries. A single archive, in the case of AIX, contains both 32 bit and 64 bit objects and shared objects. Darwin places the two shared objects into another special file, but it's the same effect. On AIX there is (or should be) one libstdc++.a, for both ppc32 and ppc64. (pthreads suppose is a separate multilib axis.) The fact that GCC historically uses multilibs is a problem. Also, when AIX added 64 bit multilibs, 32 bit was not defined as its own multilib separate from the "native" library. There historically has been a ppc64 multilib subdirectory but not a ppc32 multilib subdirectory. GCC on AIX now is being enhanced so that GCC itself can be built as a 32 bit or 64 bit application. This means that the "native" library is either 32 bit for GCC32 or 64 bit for GCC64. Ideally GCC32 and GCC64 should be able to create 32 bit and 64 bit applications that interoperate, but because of the multilib differences, they look in different locations for libraries. If GCC followed normal AIX behavior and placed all 32 bit and 64 bit shared objects into the same archive and same directory, the interoperability issue would not exist. Currently GCC32 apps look in the top-level directory and GCC32 -m64 look in the ppc64 multilib, but GCC64 apps look in the top-level directory and GCC64 -m32 apps look in the ppc32 multilib. Depending on which toolchain is used to build a 32 bit or 64 bit application, it will look for shared objects in different locations. AIX does not have a /lib64 or /lib32 directory, there only is /lib because both object modes can co-exist. The effort last year builds all of the multilibs but places the objects and shared objects into the same archive. And changes to MULTILIB_MATCHES teaches GCC to look for both multilibs in the same directory, matching AIX behavior. The remaining issue is GCC also looks for headers in the same relative multilib directories. Instructing GCC to look in the "correct" place for the libraries causes GCC to look in the "wrong" place for headers. As I replied to Marc's message, changing the GCC driver to separate library search behavior and header search behavior is fraught with danger. 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. Because of the manner in which c++config.h is built, it currently does not provide an easy mechanism to override the libstdc++ configure definitions. My approach has been to propose a few, localized changes to the libstdc++ headers so that the __int128 and __int64_t decisions are made at compile time based on the way in which GCC was invoked instead of encoded in the multilib directory hierarchy. With this change, it doesn't matter which multilib version of the libstdc++ header files are chosen because they always behave correctly. And from a design standpoint, it seems more reasonable for libstdc++ headers to adapt to the information known to the compiler instead of redundantly storing the information in a static manner. I can create a set of patches so that c++config.h includes another header after it's mangled version of configure config.h that overrides the troublesome values, but that seems like it compounds a bad design with a kludge. I am proposing a few, small changes to the libstdc++ headers so that the gratuitous differences between -m32 and -m64 are determined from the compiler when the headers are used to compile an application or library instead of hard coded into the multilib directory hierarchy. Thanks, David ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 23:01 ` David Edelsohn @ 2021-01-06 23:39 ` Jakub Jelinek 2021-01-06 23:45 ` Jakub Jelinek 0 siblings, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2021-01-06 23:39 UTC (permalink / raw) To: David Edelsohn Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Wed, Jan 06, 2021 at 06:01:23PM -0500, David Edelsohn wrote: > You continue to not respond to the actual patch and to raise issues > that don't exist in the actual patch. We are talking past each other. Consider an OS that has in stdint.h typedef long long int64_t; supports 32-bit and 64-bit multilibs and has the usual type sizes, i.e. 32-bit int, 32/64-bit long and 64-bit long long. On such a target, libstdc++ configury will define _GLIBCXX_HAVE_INT64_T_LONG_LONG for both 32-bit and 64-bit multilib, and without your patch will typedef long long streamoff; for both the multilibs, so e.g. void bar (streamoff) {} will mangle as _Z3barx on both. Now, with your patch, _GLIBCXX_HAVE_INT64_T_LONG_LONG is defined, but on the 64-bit multilib __SIZEOF_LONG__ is 8 and so you instead typedef long streamoff; for the 64-bit multilib (and keep typedef long long streamoff; for the 32-bit one). The function that previously mangled _Z3barx now mangles _Z3barl, so the ABI broke. Anyway, I'll defer to Jonathan who is the libstdc++ maintainer. Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 23:39 ` Jakub Jelinek @ 2021-01-06 23:45 ` Jakub Jelinek 2021-01-07 0:41 ` David Edelsohn 0 siblings, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2021-01-06 23:45 UTC (permalink / raw) To: David Edelsohn Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Thu, Jan 07, 2021 at 12:39:39AM +0100, Jakub Jelinek wrote: > We are talking past each other. > > Consider an OS that has in stdint.h > typedef long long int64_t; And, from grepping INT64_TYPE in config/* config/*/* it isn't just theoretic, Darwin and OpenBSD behave that way. Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-06 23:45 ` Jakub Jelinek @ 2021-01-07 0:41 ` David Edelsohn 2021-01-08 18:37 ` Jonathan Wakely 0 siblings, 1 reply; 17+ messages in thread From: David Edelsohn @ 2021-01-07 0:41 UTC (permalink / raw) To: Jakub Jelinek Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT 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? Thanks, David On Wed, Jan 6, 2021 at 6:45 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Jan 07, 2021 at 12:39:39AM +0100, Jakub Jelinek wrote: > > We are talking past each other. > > > > Consider an OS that has in stdint.h > > typedef long long int64_t; > > And, from grepping INT64_TYPE in config/* config/*/* > it isn't just theoretic, Darwin and OpenBSD behave that way. > > Jakub > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-07 0:41 ` David Edelsohn @ 2021-01-08 18:37 ` Jonathan Wakely 2021-01-08 18:52 ` Jakub Jelinek 2021-04-29 20:06 ` David Edelsohn 0 siblings, 2 replies; 17+ messages in thread From: Jonathan Wakely @ 2021-01-08 18:37 UTC (permalink / raw) To: David Edelsohn Cc: Jakub Jelinek, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT [-- Attachment #1: Type: text/plain, Size: 1432 bytes --] 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 <stdint.h>, but if that was the case then any ABI break it caused happened years ago. [-- Attachment #2: patch.txt --] [-- Type: text/x-patch, Size: 4183 bytes --] diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index e4175ea3e64..f13c5d2467f 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 <stdint.h>, 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 <stdint.h>], - [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 <stdint.h>.]) - 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 <stdint.h> - template<typename, typename> struct same { enum { value = -1 }; }; - template<typename Tp> struct same<Tp, Tp> { enum { value = 1 }; }; - int array[same<int64_t, long>::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 <stdint.h> - template<typename, typename> struct same { enum { value = -1 }; }; - template<typename Tp> struct same<Tp, Tp> { enum { value = 1 }; }; - int array[same<int64_t, long long>::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..a816ff79d16 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 718ff44628c..d2fbfc35dee 100644 --- a/libstdc++-v3/include/bits/postypes.h +++ b/libstdc++-v3/include/bits/postypes.h @@ -39,32 +39,6 @@ #include <cwchar> // For mbstate_t -// XXX If <stdint.h> is really needed, make sure to define the macros -// before including it, in order not to break <tr1/cstdint> (and <cstdint> -// 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 <stdint.h> // 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-08 18:37 ` Jonathan Wakely @ 2021-01-08 18:52 ` Jakub Jelinek 2021-01-08 20:35 ` David Edelsohn 2021-04-29 20:06 ` David Edelsohn 1 sibling, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2021-01-08 18:52 UTC (permalink / raw) To: Jonathan Wakely Cc: David Edelsohn, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Fri, Jan 08, 2021 at 06:37:03PM +0000, Jonathan Wakely wrote: > 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 <stdint.h>, but if that was the > case then any ABI break it caused happened years ago. Does clang and ICC define __INT64_TYPE__ (at least on most architectures) and does it match what gcc defines it to? Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-08 18:52 ` Jakub Jelinek @ 2021-01-08 20:35 ` David Edelsohn 0 siblings, 0 replies; 17+ messages in thread From: David Edelsohn @ 2021-01-08 20:35 UTC (permalink / raw) To: Jakub Jelinek, Jonathan Wakely Cc: Jonathan Wakely, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Fri, Jan 8, 2021 at 1:52 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Fri, Jan 08, 2021 at 06:37:03PM +0000, Jonathan Wakely wrote: > > 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 <stdint.h>, but if that was the > > case then any ABI break it caused happened years ago. > > Does clang and ICC define __INT64_TYPE__ (at least on most architectures) > and does it match what gcc defines it to? Clang (at least back to 3.0.0) and ICC (at least back to 16.0.0) define __INT64_TYPE__. If the value is not compatible with the target __int64_t type (matching GCC), there presumably are deeper problems. Thanks, David ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-01-08 18:37 ` Jonathan Wakely 2021-01-08 18:52 ` Jakub Jelinek @ 2021-04-29 20:06 ` David Edelsohn 2021-04-29 20:23 ` Jonathan Wakely 2021-04-30 19:31 ` Jonathan Wakely 1 sibling, 2 replies; 17+ messages in thread From: David Edelsohn @ 2021-04-29 20:06 UTC (permalink / raw) To: Jonathan Wakely Cc: Jakub Jelinek, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Fri, Jan 8, 2021 at 1:37 PM Jonathan Wakely <jwakely@redhat.com> 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 <stdint.h>, 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__ . Thanks, David ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-04-29 20:06 ` David Edelsohn @ 2021-04-29 20:23 ` Jonathan Wakely 2021-04-30 19:31 ` Jonathan Wakely 1 sibling, 0 replies; 17+ messages in thread From: Jonathan Wakely @ 2021-04-29 20:23 UTC (permalink / raw) To: David Edelsohn Cc: Jakub Jelinek, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On 29/04/21 16:06 -0400, David Edelsohn wrote: >On Fri, Jan 8, 2021 at 1:37 PM Jonathan Wakely <jwakely@redhat.com> 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 <stdint.h>, 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__ . Yes, I'll push that tomorrow. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-04-29 20:06 ` David Edelsohn 2021-04-29 20:23 ` Jonathan Wakely @ 2021-04-30 19:31 ` Jonathan Wakely 2021-04-30 20:18 ` David Edelsohn 1 sibling, 1 reply; 17+ messages in thread From: Jonathan Wakely @ 2021-04-30 19:31 UTC (permalink / raw) To: David Edelsohn Cc: Jakub Jelinek, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT [-- Attachment #1: Type: text/plain, Size: 1964 bytes --] On 29/04/21 16:06 -0400, David Edelsohn wrote: >On Fri, Jan 8, 2021 at 1:37 PM Jonathan Wakely <jwakely@redhat.com> 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 <stdint.h>, 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. [-- Attachment #2: patch.txt --] [-- Type: text/x-patch, Size: 6428 bytes --] commit 7ddcd26ebb60a8a4b57330442631115cabb6ec22 Author: Jonathan Wakely <jwakely@redhat.com> 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 <dje.gcc@gmail.com> 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 <stdint.h> 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 <stdint.h>, 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 <stdint.h>], - [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 <stdint.h>.]) - 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 <stdint.h> - template<typename, typename> struct same { enum { value = -1 }; }; - template<typename Tp> struct same<Tp, Tp> { enum { value = 1 }; }; - int array[same<int64_t, long>::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 <stdint.h> - template<typename, typename> struct same { enum { value = -1 }; }; - template<typename Tp> struct same<Tp, Tp> { enum { value = 1 }; }; - int array[same<int64_t, long long>::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 <cwchar> // For mbstate_t -// XXX If <stdint.h> is really needed, make sure to define the macros -// before including it, in order not to break <tr1/cstdint> (and <cstdint> -// 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 <stdint.h> // 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T 2021-04-30 19:31 ` Jonathan Wakely @ 2021-04-30 20:18 ` David Edelsohn 0 siblings, 0 replies; 17+ messages in thread From: David Edelsohn @ 2021-04-30 20:18 UTC (permalink / raw) To: Jonathan Wakely Cc: Jakub Jelinek, Iain Sandoe, libstdc++, GCC Patches, CHIGOT, CLEMENT On Fri, Apr 30, 2021 at 3:31 PM Jonathan Wakely <jwakely@redhat.com> wrote: > > On 29/04/21 16:06 -0400, David Edelsohn wrote: > >On Fri, Jan 8, 2021 at 1:37 PM Jonathan Wakely <jwakely@redhat.com> 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 <stdint.h>, 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. Hi, Jonathan Thanks very much! It's very helpful to remove the multilib differences. I'll follow up about the int128 change in a separate email. Thanks, David ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-04-30 20:18 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-06 18:38 [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T David Edelsohn 2021-01-06 18:55 ` Marc Glisse 2021-01-06 19:11 ` David Edelsohn 2021-01-06 19:37 ` Jakub Jelinek 2021-01-06 21:20 ` David Edelsohn 2021-01-06 21:41 ` Jakub Jelinek 2021-01-06 23:01 ` David Edelsohn 2021-01-06 23:39 ` Jakub Jelinek 2021-01-06 23:45 ` Jakub Jelinek 2021-01-07 0:41 ` David Edelsohn 2021-01-08 18:37 ` Jonathan Wakely 2021-01-08 18:52 ` Jakub Jelinek 2021-01-08 20:35 ` David Edelsohn 2021-04-29 20:06 ` David Edelsohn 2021-04-29 20:23 ` Jonathan Wakely 2021-04-30 19:31 ` Jonathan Wakely 2021-04-30 20:18 ` David Edelsohn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).