diff mbox series

Ping: [PATCH] recog: Disallow subregs in mode-punned value [PR115881]

Message ID mpth6c8v5y8.fsf@arm.com
State New
Headers show
Series Ping: [PATCH] recog: Disallow subregs in mode-punned value [PR115881] | expand

Commit Message

Richard Sandiford July 29, 2024, 10:38 a.m. UTC 
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 mbox series

Patch

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);
+}