| Message ID | CAGWvnyk0q7RSgiwKr-e5R002JRrq+udBCNdaukWHRD-3L=DWgA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [libstdc++] GLIBCXX_HAVE_INT64_T | expand |
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.
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
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
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 >
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
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
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
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
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
>
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.
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
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
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
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.
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.
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
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