public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] lra: Replace subregs in bare uses & clobbers [PR108681]
Date: Tue, 07 Feb 2023 10:29:26 +0000	[thread overview]
Message-ID: <mptr0v17oqh.fsf@arm.com> (raw)

In this PR we had a write to one vector of a 4-vector tuple.
The vector had mode V1DI, and the target doesn't provide V1DI
moves, so this was converted into:

    (clobber (subreg:V1DI (reg/v:V4x1DI 92 [ b ]) 24))

followed by a DImode move.  (The clobber isn't really necessary
or helpful for a single word, but would be for wider moves.)

The subreg in the clobber survived until after RA:

    (clobber (subreg:V1DI (reg/v:V4x1DI 34 v2 [orig:92 b ] [92]) 24))

IMO this isn't well-formed.  If a subreg of a hard register simplifies
to a hard register, it should be replaced by the hard register.  If the
subreg doesn't simplify, then target-independent code can't be sure
which parts of the register are affected and which aren't.  A clobber
of such a subreg isn't useful and (again IMO) should just be removed.
Conversely, a use of such a subreg is effectively a use of the whole
inner register.

LRA has code to simplify subregs of hard registers, but it didn't
handle bare uses and clobbers.  The patch extends it to do that.

One question was whether the final_p argument to alter_subregs
should be true or false.  True is IMO dangerous, since it forces
replacements that might not be valid from a dataflow perspective,
and uses and clobbers only exist for dataflow.  As said above,
I think the correct way of handling a failed simplification would
be to delete clobbers and replace uses of subregs with uses of
the inner register.  But I didn't want to write untested code
to do that.

In the PR, the clobber caused an infinite loop in DCE, because
of a disagreement about what effect the clobber had.  But for
the reasons above, I think that was GIGO rather than a bug in
DF or DCE.

Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?

Richard


gcc/
	PR rtl-optimization/108681
	* lra-spills.cc (lra_final_code_change): Extend subreg replacement
	code to handle bare uses and clobbers.

gcc/testsuite/
	PR rtl-optimization/108681
	* gcc.target/aarch64/pr108681.c: New test.
---
 gcc/lra-spills.cc                           |  3 +++
 gcc/testsuite/gcc.target/aarch64/pr108681.c | 15 +++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr108681.c

diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index a8d7e60acd3..4af85c49d43 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -860,6 +860,9 @@ lra_final_code_change (void)
 		lra_update_dup (id, i);
 		insn_change_p = true;
 	      }
+	  if ((GET_CODE (pat) == USE || GET_CODE (pat) == CLOBBER)
+	      && alter_subregs (&XEXP (pat, 0), false))
+	    insn_change_p = true;
 	  if (insn_change_p)
 	    lra_update_operator_dups (id);
 
diff --git a/gcc/testsuite/gcc.target/aarch64/pr108681.c b/gcc/testsuite/gcc.target/aarch64/pr108681.c
new file mode 100644
index 00000000000..2391eaac2f2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr108681.c
@@ -0,0 +1,15 @@
+/* { dg-options "-O" } */
+
+#pragma GCC aarch64 "arm_neon.h"
+typedef __Int64x1_t int64x1_t;
+void foo (int64x1x4_t);
+
+void
+bar (int64x1_t a)
+{
+  for (;;) {
+    int64x1x4_t b;
+    b.val[3] = a;
+    foo (b);
+  }
+}
-- 
2.25.1


             reply	other threads:[~2023-02-07 10:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-07 10:29 Richard Sandiford [this message]
2023-02-13  6:58 ` Jeff Law
2023-02-13 21:13   ` Richard Sandiford

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=mptr0v17oqh.fsf@arm.com \
    --to=richard.sandiford@arm.com \
    --cc=gcc-patches@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).