public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Edelsohn <dje.gcc@gmail.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Jonathan Wakely <jwakely@redhat.com>,
	Iain Sandoe <iain@sandoe.co.uk>,
	 "libstdc++" <libstdc++@gcc.gnu.org>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
	 "CHIGOT, CLEMENT" <clement.chigot@atos.net>
Subject: Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
Date: Wed, 6 Jan 2021 18:01:23 -0500	[thread overview]
Message-ID: <CAGWvnynpjYY8cy4VRdn_9nvnFdi92DJbOkhvJ4tUeAB_Txvftg@mail.gmail.com> (raw)
In-Reply-To: <20210106214159.GF725145@tucnak>

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

  reply	other threads:[~2021-01-06 23:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-06 18:38 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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGWvnynpjYY8cy4VRdn_9nvnFdi92DJbOkhvJ4tUeAB_Txvftg@mail.gmail.com \
    --to=dje.gcc@gmail.com \
    --cc=clement.chigot@atos.net \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=iain@sandoe.co.uk \
    --cc=jakub@redhat.com \
    --cc=jwakely@redhat.com \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).