From: Richard Sandiford <richard.sandiford@arm.com>
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] recog: Disallow subregs in mode-punned value [PR115881]
Date: Fri, 19 Jul 2024 18:37:37 +0100 [thread overview]
Message-ID: <mptsew55llq.fsf@arm.com> (raw)
In g:9d20529d94b23275885f380d155fe8671ab5353a, I'd extended
insn_propagation to handle simple cases of hard-reg mode punning.
The punned "to" value was created using simplify_subreg rather
than simplify_gen_subreg, on the basis that hard-coded subregs
aren't generally useful after RA (where hard-reg propagation is
expected to happen).
This PR is about a case where the subreg gets pushed into the
operands of a plus, but the subreg on one of the operands
cannot be simplified. Specifically, we have to generate
(subreg:SI (reg:DI sp) 0) rather than (reg:SI sp), since all
references to the stack pointer must be via stack_pointer_rtx.
However, code in x86 (reasonably) expects no subregs of registers
to appear after RA, except for special cases like strict_low_part.
This leads to an awkward situation where we can't ban subregs of sp
(because of the strict_low_part use), can't allow direct references
to sp in other modes (because of the stack_pointer_rtx requirement),
and can't allow rvalue uses of the subreg (because of the "no subregs
after RA" assumption). It all seems a bit of a mess...
I sat on this for a while in the hope that a clean solution might
become apparent, but in the end, I think we'll just have to check
manually for nested subregs and punt on them.
Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install?
Richard
gcc/
PR rtl-optimization/115881
* recog.cc: Include rtl-iter.h.
(insn_propagation::apply_to_rvalue_1): Check that the result
of simplify_subreg does not include nested subregs.
gcc/tetsuite/
PR rtl-optimization/115881
* cc.c-torture/compile/pr115881.c: New test.
---
gcc/recog.cc | 21 +++++++++++++++++++
.../gcc.c-torture/compile/pr115881.c | 16 ++++++++++++++
2 files changed, 37 insertions(+)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr115881.c
diff --git a/gcc/recog.cc b/gcc/recog.cc
index 54b317126c2..23e4820180f 100644
--- a/gcc/recog.cc
+++ b/gcc/recog.cc
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see
#include "reload.h"
#include "tree-pass.h"
#include "function-abi.h"
+#include "rtl-iter.h"
#ifndef STACK_POP_CODE
#if STACK_GROWS_DOWNWARD
@@ -1082,6 +1083,7 @@ insn_propagation::apply_to_rvalue_1 (rtx *loc)
|| !REG_CAN_CHANGE_MODE_P (REGNO (x), GET_MODE (from),
GET_MODE (x)))
return false;
+
/* If the reference is paradoxical and the replacement
value contains registers, we would need to check that the
simplification below does not increase REG_NREGS for those
@@ -1090,11 +1092,30 @@ insn_propagation::apply_to_rvalue_1 (rtx *loc)
if (paradoxical_subreg_p (GET_MODE (x), GET_MODE (from))
&& !CONSTANT_P (to))
return false;
+
newval = simplify_subreg (GET_MODE (x), to, GET_MODE (from),
subreg_lowpart_offset (GET_MODE (x),
GET_MODE (from)));
if (!newval)
return false;
+
+ /* Check that the simplification didn't just push an explicit
+ subreg down into subexpressions. In particular, for a register
+ R that has a fixed mode, such as the stack pointer, a subreg of:
+
+ (plus:M (reg:M R) (const_int C))
+
+ would be:
+
+ (plus:N (subreg:N (reg:M R) ...) (const_int C'))
+
+ But targets can legitimately assume that subregs of hard registers
+ will not be created after RA (except in special circumstances,
+ such as strict_low_part). */
+ subrtx_iterator::array_type array;
+ FOR_EACH_SUBRTX (iter, array, newval, NONCONST)
+ if (GET_CODE (*iter) == SUBREG)
+ return false;
}
if (should_unshare)
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115881.c b/gcc/testsuite/gcc.c-torture/compile/pr115881.c
new file mode 100644
index 00000000000..8379704c4c8
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr115881.c
@@ -0,0 +1,16 @@
+typedef unsigned u32;
+int list_is_head();
+void tu102_acr_wpr_build_acr_0_0_0(int, long, u32);
+void tu102_acr_wpr_build() {
+ u32 offset = 0;
+ for (; list_is_head();) {
+ int hdr;
+ u32 _addr = offset, _size = sizeof(hdr), *_data = &hdr;
+ while (_size--) {
+ tu102_acr_wpr_build_acr_0_0_0(0, _addr, *_data++);
+ _addr += 4;
+ }
+ offset += sizeof(hdr);
+ }
+ tu102_acr_wpr_build_acr_0_0_0(0, offset, 0);
+}
--
2.25.1
next reply other threads:[~2024-07-19 17:37 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-19 17:37 Richard Sandiford [this message]
2024-07-29 10:38 ` Ping: " Richard Sandiford
2024-07-30 21:04 ` Jeff Law
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=mptsew55llq.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).