public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Peter Bergner <bergner@linux.ibm.com>
Cc: richard.sandiford@arm.com, Gcc-patches <gcc-patches@gcc.gnu.org>,
	"ian@airs.com" <ian@airs.com>
Subject: Re: [PATCH] lower-subreg: PR94123, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail
Date: Mon, 30 Mar 2020 11:39:02 -0500	[thread overview]
Message-ID: <20200330163902.GT22482@gate.crashing.org> (raw)
In-Reply-To: <6c43dfe0-2dd8-aa95-4ace-62b49657e0aa@linux.ibm.com>

On Mon, Mar 30, 2020 at 11:23:12AM -0500, Peter Bergner wrote:
> On 3/30/20 6:26 AM, Segher Boessenkool wrote:
> > On Mon, Mar 30, 2020 at 09:50:05AM +0100, Richard Sandiford wrote:
> >> Peter Bergner via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >>> -  if (HARD_REGISTER_NUM_P (rd) || HARD_REGISTER_NUM_P (rs))
> >>> +  if (HARD_REGISTER_NUM_P (rd))
> >>>      return false;
> >>>  
> >>>    b = reg_copy_graph[rs];
> >>
> >> I guess this would also work if we dropped the rd check instead.
> >> So how about s/||/&&/ instead, to avoid the assymetry?
> >>
> >> I agree something like this is a better fix long-term, since we
> >> shouldn't be relying on make_more_copies outside combine.
> > 
> > Yes; on the other hand, most RTL passes should do something to not have
> > hard registers forwarded into non-move instructions (where they can
> > cause problems later).  (make_more_copies itself is a technicality
> > specific to how combine works, and we might be able to drop it in the
> > future).
> 
> I kind of agree with Richard above on making it more applicable/symmetric,
> but why can't we just remove the HARD_REGISTER_NUM_P() tests altogether?
> It's not like lower-subreg is extending hard register lifetime usage than
> what is already there in the rtl.  We're just decomposing what's already
> there into smaller register sized chunks.  Is there a problem with that
> I'm not aware of?

The function comment is (since day 1):
  /* If SET is a copy from one multi-word pseudo-register to another,
     record that in reg_copy_graph.  Return whether it is such a
     copy.  */
so a) that needs fixing; and b) what does this change for the (single)
caller of find_pseudo_copy?  The comment there isn't very enlightening:
  /* We mark pseudo-to-pseudo copies as decomposable during the
     second pass only.  The first pass is so early that there is
     good chance such moves will be optimized away completely by
     subsequent optimizations anyway.

     However, we call find_pseudo_copy even during the first pass
     so as to properly set up the reg_copy_graph.  */

(The function *name* should change as well, if you make this change).


Segher

  parent reply	other threads:[~2020-03-30 16:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27 22:41 Peter Bergner
2020-03-27 23:26 ` Ian Lance Taylor
2020-03-28 19:22 ` Segher Boessenkool
2020-03-28 23:39   ` Peter Bergner
2020-04-02 21:56     ` Segher Boessenkool
2020-03-30  8:50 ` Richard Sandiford
2020-03-30 11:26   ` Segher Boessenkool
2020-03-30 16:23     ` Peter Bergner
2020-03-30 16:26       ` Peter Bergner
2020-03-30 16:39       ` Segher Boessenkool [this message]
2020-03-30 16:06   ` Segher Boessenkool
2020-04-01 17:48   ` Peter Bergner
2020-04-01 18:32     ` Richard Sandiford
2020-04-01 19:43       ` Peter Bergner

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=20200330163902.GT22482@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=bergner@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=ian@airs.com \
    --cc=richard.sandiford@arm.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).