Alex Coplan <alex.cop...@arm.com> writes:
> Hi,
>
> The PR shows two different cases where try_promote_writeback produces an
> RTL pattern which isn't recognized. Currently this leads to an ICE, as
> we assert recog success, but I think it's better just to back out of the
> changes gracefully if recog fails (as we do in the main fuse_pair case).
>
> In theory since we check the ranges here recog shouldn't fail (which is
> why I had the assert in the first place), but the PR shows an edge case
> in the patterns where if we form a pre-writeback pair where the
> writeback offset is exactly -S, where S is the size in bytes of one
> transfer register, we fail to match the expected pattern as the patterns
> look explicitly for plus operands in the mems. I think fixing this
> would require adding at least four new special-case patterns to
> aarch64.md for what doesn't seem to be a particularly useful variant of
> the insns. Even if we were to do that, I think it would be GCC 15
> material, and it's better to just punt for GCC 14.
>
> The ILP32 case in the PR is a bit different, as that shows us trying to
> combine a pair with DImode base register operands in the mems together
> with an SImode trailing update of the base register. This leads to us
> forming an RTL pattern which references the base register in both SImode
> and DImode, which also fails to recog. Again, I think it's best just to
> take the missed optimization for now. If we really want to make this
> (try_promote_writeback) work for ILP32, we can try to do it for GCC 15.
>
> Bootstrapped/regtested on aarch64-linux-gnu (with/without passes
> enabled), OK for trunk?
>
> Thanks,
> Alex
>
> gcc/ChangeLog:
>
> PR target/113114
> * config/aarch64/aarch64-ldp-fusion.cc (try_promote_writeback):
> Don't assert recog success, just punt if the writeback pair
> isn't recognized.
>
> gcc/testsuite/ChangeLog:
>
> PR target/113114
> * gcc.c-torture/compile/pr113114.c: New test.
> * gcc.target/aarch64/pr113114.c: New test.
OK, thanks.
Richard
>
> diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc
> b/gcc/config/aarch64/aarch64-ldp-fusion.cc
> index 689a8c884bd..19142153f41 100644
> --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc
> +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc
> @@ -2672,7 +2672,15 @@ try_promote_writeback (insn_info *insn)
> for (unsigned i = 0; i < ARRAY_SIZE (changes); i++)
> gcc_assert (rtl_ssa::restrict_movement_ignoring (*changes[i],
> is_changing));
>
> - gcc_assert (rtl_ssa::recog_ignoring (attempt, pair_change, is_changing));
> + if (!rtl_ssa::recog_ignoring (attempt, pair_change, is_changing))
> + {
> + if (dump_file)
> + fprintf (dump_file, "i%d: recog failed on wb pair, bailing out\n",
> + insn->uid ());
> + cancel_changes (0);
> + return;
> + }
> +
> gcc_assert (crtl->ssa->verify_insn_changes (changes));
> confirm_change_group ();
> crtl->ssa->change_insns (changes);
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr113114.c
> b/gcc/testsuite/gcc.c-torture/compile/pr113114.c
> new file mode 100644
> index 00000000000..978e594eb3d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr113114.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-funroll-loops" } */
> +float val[128];
> +float x;
> +void bar() {
> + int i = 55;
> + for (; i >= 0; --i)
> + x += val[i];
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr113114.c
> b/gcc/testsuite/gcc.target/aarch64/pr113114.c
> new file mode 100644
> index 00000000000..5b0383c2435
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr113114.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mabi=ilp32 -O -mearly-ldp-fusion -mlate-ldp-fusion" } */
> +void foo_n(double *a) {
> + int i = 1;
> + for (; i < (int)foo_n; i++)
> + a[i] = a[i - 1] + a[i + 1] * a[i];
> +}
