public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Michael Meissner <meissner@linux.vnet.ibm.com>
To: David Edelsohn <dje.gcc@gmail.com>
Cc: Michael Meissner <meissner@linux.vnet.ibm.com>,
	gcc-patches@gcc.gnu.org,         Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH] Fix builtin attributes on powerpc
Date: Wed, 14 Oct 2009 15:21:00 -0000	[thread overview]
Message-ID: <20091014151518.GA8113@hungry-tiger.westford.ibm.com> (raw)
In-Reply-To: <303e1d290910131702r13407263i1b226dcad22dddc7@mail.gmail.com>

On Tue, Oct 13, 2009 at 08:02:04PM -0400, David Edelsohn wrote:
> On Tue, Oct 13, 2009 at 4:25 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> 
> > It is too late for GCC 4.5 to do things in a common fashion.  The real solution
> > is to move the backend builtins into the main builtins.  I had some patches to
> > allow backends to optionally do this, and when 4.6 opens up we can explore
> > this.
> 
> The question is whether Jakub and other GCC port maintainers agree
> that this is the direction to go in the future, not that it is
> implemented in all targets for GCC 4.5.  I want to see some consensus
> before this patch is committed.
> 
> > I agree MAX_RS6000_BUILTINS is more consistant, but RS6000_BUILTIN_COUNT is
> > what was originally used, so I didn't change that.
> 
> Please make the change as part of the patch.

Unfortunately, I'm not sure I can do all of this in the time remaining.  I
suspect it will take at least 5 days of time to build the infrastructure to do
what rth and richi have asked for.  The goals as I see it are:

  1) Move the creation of the MD builtins to be part of the normal builtin
     creation, in a seamless fashion without forcing the backends to have a
     flag day changing their builtin handling (originally from rth).  Part of
     this was done in this RFC patch that provided the new infrastructure:
     http://gcc.gnu.org/ml/gcc-patches/2009-09/msg01820.html

  2) Allow for lazy creation of the trees for builtin functions and types
     (originally from richi).  I know where the places that need to be modified
     are, but it will involve changes in both the front ends and middle end to
     do this.  This will take some amount of time to get right.

  3) Move all of the rs6000 builtins (the various bdesc tables in rs6000.c) to
     this new scheme.  Just due to the number of builtins that exist in the
     rs6000 backend, this will take some time to make sure all of the types,
     etc. are properly created.

  4) Then possibly get x86, arm, etc. to move to the new scheme.

However, I can't do #3 until at least the infrastructure changes are in place,
and given we are in stage 3 right now, I don't think this is the time for doing
it.  Thus the 2 different patches that I have submitted are a stopgap measure
for the 4.5 time frame, until the infrastructure is done in the 4.6 time frame.
One patch is just a big switch statement that identifies the problematical
builtins, and the second patch just creates a .def file to define both the enum
id, and attributes to use.  To me they are semantically identical, and I can
easily go back to the switch statement if the issue is the creation of a new
file is the issue.

Right now, I have several codegen issues in the power7 code that need to be
fixed before 4.5 goes out, and I judge that these are more important than
getting the attributes correct, given the 4.4 backport has worked around the
problem that started this set of patches.

I can spend the odd hour here and there making the patch more acceptable, but I
don't feel I can spend the week or two of time right now doing the scheme the
right way so that it won't have to be reworked in the 4.6 time frame and
possibly send out 4.5 where several benchmarks are getting miscompiled.  It
would be nice to get the power backend to have pairity with the x86 to at least
set NOTHROW and READONLY where appropriate.

I wish I had known about the issue earlier and could have gotten the
infrastructure in before stage1 closed, but that's the breaks.

-- 
Michael Meissner, IBM
4 Technology Place Drive, MS 2203A, Westford, MA, 01886, USA
meissner@linux.vnet.ibm.com

  parent reply	other threads:[~2009-10-14 15:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-01 18:58 Michael Meissner
2009-10-01 19:01 ` Andrew Pinski
2009-10-01 19:14   ` Michael Meissner
2009-10-13 13:36 ` David Edelsohn
2009-10-13 20:40   ` Michael Meissner
2009-10-14  0:02     ` David Edelsohn
2009-10-14  9:58       ` Richard Guenther
2009-10-14 15:21       ` Michael Meissner [this message]
2009-10-14 16:19         ` Richard Guenther
2009-10-14 20:08         ` David Edelsohn
2009-10-14 22:21           ` Michael Meissner
2009-10-15 20:59           ` [PATCH, committed] PR target/23983, fix " Michael Meissner
2009-10-19 22:24             ` Jakub Jelinek
2009-10-20 16:29               ` Jakub Jelinek
2009-10-20 17:56                 ` 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=20091014151518.GA8113@hungry-tiger.westford.ibm.com \
    --to=meissner@linux.vnet.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /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).