public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: Ajit Agarwal <aagarwa1@linux.ibm.com>
Cc: Alex Coplan <alex.coplan@arm.com>,
	 "Kewen.Lin" <linkw@linux.ibm.com>,
	 Segher Boessenkool <segher@kernel.crashing.org>,
	 Michael Meissner <meissner@linux.ibm.com>,
	 Peter Bergner <bergner@linux.ibm.com>,
	 David Edelsohn <dje.gcc@gmail.com>,
	 gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Patch, rs6000, middle-end] v2: Add implementation for different targets for pair mem fusion
Date: Fri, 14 Jun 2024 11:56:37 +0100	[thread overview]
Message-ID: <mptv82bpzbu.fsf@arm.com> (raw)
In-Reply-To: <870719f4-0923-497d-bba2-c83e46a4b13a@linux.ibm.com> (Ajit Agarwal's message of "Fri, 14 Jun 2024 16:03:06 +0530")

Ajit Agarwal <aagarwa1@linux.ibm.com> writes:
> Hello Richard:
>
> All comments are addressed.

I don't think this addresses the following comments from the previous
reviews:

(1) It is not correct to mark existing insn uses as live-out.
    The patch mustn't try to do this.

(2) To quote a previous review:

    It's probably better to create a fresh OO register, rather than
    change an existing 128-bit register to 256 bits.  If we do that,
    and if reg:V16QI 125 is the destination of the second load
    (which I assume it is from the 16 offset in the subreg),
    then the new RTL should be:

      (vec_select:HI (subreg:V8HI (reg:OO NEW_REG) 16) ...)

    It's possible to get this by using insn_propagation to replace
    (reg:V16QI 125) with (subreg:V16QI (reg:OO NEW_REG) 16).
    insn_propagation should then take care of the rest.

    There are no existing rtl-ssa routines for handling new registers
    though.  (The idea was to add things as the need arose.)

The reason for (2) is that changing the mode of an existing pseudo
invalidates all existing references to that pseudo.  Although the
patch tries to fix things up, it's doing that at a stage where
there is already "garbage in" (in the sense that the starting
RTL is invalid).  Just changing the mode would also invalidate
things like REG_EXPR, for example.

In contrast, the advantage of creating a new pseudo means that every
insn transformation is from structurally valid RTL to structurally
valid RTL.  It also prevents information being incorrectly carried
over from the old pseudo.
x
Thanks,
Richard

       reply	other threads:[~2024-06-14 10:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <870719f4-0923-497d-bba2-c83e46a4b13a@linux.ibm.com>
2024-06-14 10:56 ` Richard Sandiford [this message]
2024-06-18 16:47   ` Ajit Agarwal
2024-06-18 20:31     ` Richard Sandiford
2024-06-19  7:22       ` Ajit Agarwal
2024-06-19  7:57         ` Ajit Agarwal
2024-06-19  8:24         ` Richard Sandiford
2024-06-19  9:06           ` Ajit Agarwal
2024-06-19  9:10             ` Richard Sandiford
2024-06-19  9:18               ` Ajit Agarwal
2024-06-19  9:22                 ` Richard Sandiford
2024-06-19  9:39                   ` Ajit Agarwal
2024-06-19  9:56                     ` Richard Sandiford
2024-06-20  9:32                       ` Ajit Agarwal

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=mptv82bpzbu.fsf@arm.com \
    --to=richard.sandiford@arm.com \
    --cc=aagarwa1@linux.ibm.com \
    --cc=alex.coplan@arm.com \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=linkw@linux.ibm.com \
    --cc=meissner@linux.ibm.com \
    --cc=segher@kernel.crashing.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).