https://gcc.gnu.org/g:e322dff09d011f65f5cae4e95c3a24ccfae7b1e1
commit r16-984-ge322dff09d011f65f5cae4e95c3a24ccfae7b1e1
Author: Richard Sandiford <richard.sandif...@arm.com>
Date: Fri May 30 09:36:35 2025 +0100
rtl-ssa: Reject non-address uses of autoinc regs [PR120347]
As the rtl.texi documentation of RTX_AUTOINC expressions says:
If a register used as the operand of these expressions is used in
another address in an insn, the original value of the register is
used. Uses of the register outside of an address are not permitted
within the same insn as a use in an embedded side effect expression
because such insns behave differently on different machines and hence
must be treated as ambiguous and disallowed.
late-combine was failing to follow this rule. One option would have
been to enforce it during the substitution phase, like combine does.
This could either be a dedicated condition in the substitution code
or, more generally, an extra condition in can_merge_accesses.
(The latter would include extending is_pre_post_modify to uses.)
However, since the restriction applies to patterns rather than to
actions on patterns, the more robust fix seemed to be test and reject
this case in (a subroutine of) rtl_ssa::recog. We already do something
similar for hard-coded register clobbers.
Using vec_rtx_properties isn't the lightest-weight operation
out there. I did wonder about relying on the is_pre_post_modify
flag of the definitions in the new_defs array, but that would
require callers that create new autoincs to set the flag before
calling recog. Normally these flags are instead updated
automatically based on the final pattern.
Besides, recog itself has had to traverse the whole pattern,
and it is even less light-weight than vec_rtx_properties.
At least the pattern should be in cache.
The rtl-ssa fix showed up a mistake (of mine) in the rtl_properties
walker: try_to_add_src would drop all flags except IN_NOTE before
recursing into RTX_AUTOINC addresses.
RTX_AUTOINCs only occur in addresses, and so for them, the flags coming
into try_to_add_src are set by:
unsigned int base_flags = flags & rtx_obj_flags::STICKY_FLAGS;
...
if (MEM_P (x))
{
...
unsigned int addr_flags = base_flags | rtx_obj_flags::IN_MEM_STORE;
if (flags & rtx_obj_flags::IS_READ)
addr_flags |= rtx_obj_flags::IN_MEM_LOAD;
try_to_add_src (XEXP (x, 0), addr_flags);
return;
}
This means that the only flags that can be set are:
- IN_NOTE (the sole member of STICKY_FLAGS)
- IN_MEM_STORE
- IN_MEM_LOAD
Thus dropping all flags except IN_NOTE had the effect of dropping
IN_MEM_STORE and IN_MEM_LOAD, and nothing else. But those flags
are the ones that mark something as being part of a mem address.
The exclusion was therefore exactly wrong.
gcc/
PR rtl-optimization/120347
* rtlanal.cc (rtx_properties::try_to_add_src): Don't drop the
IN_MEM_LOAD and IN_MEM_STORE flags for autoinc registers.
* rtl-ssa/changes.cc (recog_level2): Check whether an
RTX_AUTOINCed register also appears outside of an address.
gcc/testsuite/
PR rtl-optimization/120347
* gcc.dg/torture/pr120347.c: New test.
Diff:
---
gcc/rtl-ssa/changes.cc | 18 ++++++++++++++++++
gcc/rtlanal.cc | 2 +-
gcc/testsuite/gcc.dg/torture/pr120347.c | 13 +++++++++++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc
index eb579ad3ad7e..f7aa6a66cdf5 100644
--- a/gcc/rtl-ssa/changes.cc
+++ b/gcc/rtl-ssa/changes.cc
@@ -1106,6 +1106,24 @@ recog_level2 (insn_change &change, add_regno_clobber_fn
add_regno_clobber)
}
}
+ // Per rtl.texi, registers that are modified using RTX_AUTOINC operations
+ // cannot also appear outside an address.
+ vec_rtx_properties properties;
+ properties.add_pattern (pat);
+ for (rtx_obj_reference def : properties.refs ())
+ if (def.is_pre_post_modify ())
+ for (rtx_obj_reference use : properties.refs ())
+ if (def.regno == use.regno && !use.in_address ())
+ {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "register %d is both auto-modified"
+ " and used outside an address:\n", def.regno);
+ print_rtl_single (dump_file, pat);
+ }
+ return false;
+ }
+
// check_asm_operands checks the constraints after RA, so we don't
// need to do it again.
if (reload_completed && !asm_p)
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index 86a5e4733088..900f53e252a9 100644
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
@@ -2235,7 +2235,7 @@ rtx_properties::try_to_add_src (const_rtx x, unsigned int
flags)
{
has_pre_post_modify = true;
- unsigned int addr_flags = (base_flags
+ unsigned int addr_flags = (flags
| rtx_obj_flags::IS_PRE_POST_MODIFY
| rtx_obj_flags::IS_READ);
try_to_add_dest (XEXP (x, 0), addr_flags);
diff --git a/gcc/testsuite/gcc.dg/torture/pr120347.c
b/gcc/testsuite/gcc.dg/torture/pr120347.c
new file mode 100644
index 000000000000..a2d187bbc5c6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr120347.c
@@ -0,0 +1,13 @@
+/* { dg-do assemble } */
+/* { dg-additional-options "-march=armv7-a -mthumb" { target { arm_arch_v7a_ok
&& arm_thumb2_ok } } } */
+
+void *end;
+void **start;
+void main(void)
+{
+ for (; end; start++) {
+ if (*start)
+ return;
+ *start = start;
+ }
+}
