* [PATCH] recog: Disallow subregs in mode-punned value [PR115881]
@ 2024-07-19 17:37 Richard Sandiford
2024-07-29 10:38 ` Ping: " Richard Sandiford
2024-07-30 21:04 ` Jeff Law
0 siblings, 2 replies; 3+ messages in thread
From: Richard Sandiford @ 2024-07-19 17:37 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Ping: [PATCH] recog: Disallow subregs in mode-punned value [PR115881]
2024-07-19 17:37 [PATCH] recog: Disallow subregs in mode-punned value [PR115881] Richard Sandiford
@ 2024-07-29 10:38 ` Richard Sandiford
2024-07-30 21:04 ` Jeff Law
1 sibling, 0 replies; 3+ messages in thread
From: Richard Sandiford @ 2024-07-29 10:38 UTC (permalink / raw)
To: gcc-patches
Ping
Richard Sandiford <richard.sandiford@arm.com> writes:
> 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);
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] recog: Disallow subregs in mode-punned value [PR115881]
2024-07-19 17:37 [PATCH] recog: Disallow subregs in mode-punned value [PR115881] Richard Sandiford
2024-07-29 10:38 ` Ping: " Richard Sandiford
@ 2024-07-30 21:04 ` Jeff Law
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Law @ 2024-07-30 21:04 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
On 7/19/24 11:37 AM, Richard Sandiford wrote:
> 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.
OK
jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-07-30 21:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-19 17:37 [PATCH] recog: Disallow subregs in mode-punned value [PR115881] Richard Sandiford
2024-07-29 10:38 ` Ping: " Richard Sandiford
2024-07-30 21:04 ` Jeff Law
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).