Alex Coplan <alex.cop...@arm.com> writes: > This is a v2 because I accidentally sent a WIP version of the patch last > time round which used replace_equiv_address instead of > replace_equiv_address_nv; that caused some ICEs (pointed out by the > Linaro CI) since pair addressing modes aren't a subset of the addresses > that are accepted by memory_operand for a given mode. > > This patch should otherwise be identical to v1. Bootstrapped/regtested > on aarch64-linux-gnu (indeed this is the patch I actually tested last > time), is this version also OK for GCC 15?
OK, thanks. Sorry for missing this in the first review. Richard > Thanks, > Alex > > --- >8 --- > > The ldp/stp fusion pass can change the base of an access so that the two > accesses end up using a common base register. So far we have been using > adjust_address_nv to do this, but this means that we don't preserve > other properties of the mem we're replacing. It seems better to use > replace_equiv_address_nv, as this will preserve e.g. the MEM_ALIGN of the > mem whose address we're changing. > > The PR shows that by adjusting the other mem we lose alignment > information about the original access and therefore end up rejecting an > otherwise viable pair when --param=aarch64-stp-policy=aligned is passed. > This patch fixes that by using replace_equiv_address_nv instead. > > Notably this is the same approach as taken by > aarch64_check_consecutive_mems when a change of base is required, so > this at least makes things more consistent between the ldp fusion pass > and the peepholes. > > gcc/ChangeLog: > > PR target/114674 > * config/aarch64/aarch64-ldp-fusion.cc (ldp_bb_info::fuse_pair): > Use replace_equiv_address_nv on a change of base instead of > adjust_address_nv on the other access. > > gcc/testsuite/ChangeLog: > > PR target/114674 > * gcc.target/aarch64/pr114674.c: New test. > > diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc > b/gcc/config/aarch64/aarch64-ldp-fusion.cc > index 365dcf48b22..d07d79df06c 100644 > --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc > +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc > @@ -1730,11 +1730,11 @@ ldp_bb_info::fuse_pair (bool load_p, > adjust_amt *= -1; > > rtx change_reg = XEXP (change_pat, !load_p); > - machine_mode mode_for_mem = GET_MODE (change_mem); > rtx effective_base = drop_writeback (base_mem); > - rtx new_mem = adjust_address_nv (effective_base, > - mode_for_mem, > - adjust_amt); > + rtx adjusted_addr = plus_constant (Pmode, > + XEXP (effective_base, 0), > + adjust_amt); > + rtx new_mem = replace_equiv_address_nv (change_mem, adjusted_addr); > rtx new_set = load_p > ? gen_rtx_SET (change_reg, new_mem) > : gen_rtx_SET (new_mem, change_reg); > diff --git a/gcc/testsuite/gcc.target/aarch64/pr114674.c > b/gcc/testsuite/gcc.target/aarch64/pr114674.c > new file mode 100644 > index 00000000000..944784fd008 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/pr114674.c > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O3 --param=aarch64-stp-policy=aligned" } */ > +typedef struct { > + unsigned int f1; > + unsigned int f2; > +} test_struct; > + > +static test_struct ts = { > + 123, 456 > +}; > + > +void foo(void) > +{ > + ts.f2 = 36969 * (ts.f2 & 65535) + (ts.f1 >> 16); > + ts.f1 = 18000 * (ts.f2 & 65535) + (ts.f2 >> 16); > +} > +/* { dg-final { scan-assembler-times "stp" 1 } } */