On Tue, Sep 29, 2026 at 8:44 AM Richard Biener <[email protected]> wrote:
>
> For the testcase in PR127633 we combine two adjacent DFmode loads from
> the stack into a V2DFmode via a peephole2 that uses adjust_address.
> That unfortunately leaves both the MEM_EXPR and the MEM_ALIAS_SET
> unaltered causing later scheduling to move the widened load before
> the store. The fix is to instead use widen_memory_access which
> attempts to fixup MEM_EXPR and resets MEM_ALIAS_SET.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu. The
> sse init testcase adjustments are due to mdreorg no longer
> splitting the merged, possibly STLF stall causing MEMs as
> the heuristic in ix86_split_stlf_stall_load requires
>
> || !MEM_EXPR (src)
> || TREE_CODE (get_base_address (MEM_EXPR (src))) != PARM_DECL
I'd push for not fusing in the first place rather than accepting the
regression. The peephole fuses the loads, and
ix86_split_stlf_stall_load then has to split them again. The obvious
fix is a shared predicate, used by both the peephole condition and the
split heuristic, so that two adjacent loads from incoming stack
arguments aren't merged at all. That keeps the old codegen and removes
the fuse-then-split churn.
> but the previously generated
>
> (mem/c:V2DF (plus:SI (reg/f:SI 7 sp)
> (const_int 4 [0x4])) [1 x+0 S16 A32]))
>
> is of course not correct - the access does not only access 'x'
> (but also 'y'). We now generate the correct
>
> (mem/c:V2DF (plus:SI (reg/f:SI 7 sp)
> (const_int 4 [0x4])) [0 S16 A32]))
>
> the heuristic should have an alternative looking at maybe RTL
> points-to, or we should accept this "regression" or find ways
> to instead not apply the fusion in the first place (possibly
> by doing the above very same check?). The only possible
> improvement to the merged MEM is that we could preserve the
> alias-set if it is the same for both original MEMs, not sure
> if that's worth it.
Preserving it when both source MEMs share an alias set is cheap in
principle, but I'd leave it out. Alias set 0 is conservative and
correct, and the gain is small.
> OK? Any opinion on the above?
>
> Thanks,
> Richard.
>
> PR target/127633
> * config/i386/sse.md (movsd/movhpd to movupd): Use
> widen_memory_access instead of adjust_address to merge
> the MEMs.
>
> * gcc.dg/pr127633.c: New testcase.
> * gcc.target/i386/sse2-init-v2df-2.c: Adjust.
> * gcc.target/i386/sse4_1-init-v2df-2.c: Likewise.
> ---
> gcc/config/i386/sse.md | 6 +-
> gcc/testsuite/gcc.dg/pr127633.c | 68 +++++++++++++++++++
> .../gcc.target/i386/sse2-init-v2df-2.c | 6 +-
> .../gcc.target/i386/sse4_1-init-v2df-2.c | 6 +-
> 4 files changed, 79 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/pr127633.c
>
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index 0ce82789dc6..2b6d7de1f7f 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -2021,7 +2021,7 @@
> "TARGET_SSE2 && TARGET_SSE_UNALIGNED_LOAD_OPTIMAL
> && ix86_operands_ok_for_move_multiple (operands, true, DFmode)"
> [(set (match_dup 2) (match_dup 5))]
> - "operands[5] = adjust_address (operands[1], V2DFmode, 0);")
> + "operands[5] = widen_memory_access (operands[1], V2DFmode, 0);")
>
> (define_peephole2
> [(set (match_operand:DF 0 "sse_reg_operand")
> @@ -2033,7 +2033,7 @@
> && REGNO (operands[4]) == REGNO (operands[2])
> && ix86_operands_ok_for_move_multiple (operands, true, DFmode)"
> [(set (match_dup 2) (match_dup 5))]
> - "operands[5] = adjust_address (operands[1], V2DFmode, 0);")
> + "operands[5] = widen_memory_access (operands[1], V2DFmode, 0);")
>
> ;; Merge movlpd/movhpd to movupd for TARGET_SSE_UNALIGNED_STORE_OPTIMAL
> targets.
> (define_peephole2
> @@ -2046,7 +2046,7 @@
> "TARGET_SSE2 && TARGET_SSE_UNALIGNED_STORE_OPTIMAL
> && ix86_operands_ok_for_move_multiple (operands, false, DFmode)"
> [(set (match_dup 4) (match_dup 1))]
> - "operands[4] = adjust_address (operands[0], V2DFmode, 0);")
> + "operands[4] = widen_memory_access (operands[0], V2DFmode, 0);")
>
> (define_insn "<sse3>_lddqu<avxsizesuffix>"
> [(set (match_operand:VI1 0 "register_operand" "=x,x")
> diff --git a/gcc/testsuite/gcc.dg/pr127633.c b/gcc/testsuite/gcc.dg/pr127633.c
> new file mode 100644
> index 00000000000..500e84f66ff
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr127633.c
> @@ -0,0 +1,68 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target int32plus } */
> +/* { dg-options "-O3 -funroll-all-loops -fno-vect-cost-model" } */
> +
> +int d, ab = -104, ac, ad = -170, ae = -51, af = -151, ag = -1966532803, ah,
> ai,
> + aj, ak, al, am, an, *ao, v, ap, aq;
> +short w, y, z, ar, as, at, au, av, aw, ax = -156, ay, az, ba;
> +long bb = -38, bc, bd, be, bf;
> +double bg, bh, *bi;
> +char bj;
> +int a(unsigned bk, unsigned bl) {
> + return bk^bl;
> +}
This testcase has undefined behaviour. a (unsigned, unsigned) is
called with doubles such as cb[2] = -8.0, cb[1] (about -655364) and bw
(-131072). Converting a negative double to unsigned is UB because -8
is outside the representable range of unsigned. On x86 it happens to
wrap through cvttsd2si so perhaps move the testcase to
gcc.target/i386.
Uros.
> +[[gnu::noinline,gnu::noclone]]
> +int aa(short bn) {
> + return bn;
> +}
> +[[gnu::noinline,gnu::noclone]]
> +int s(long bk, short bl, short bm, long bn, int bo, short bp, int bq) {
> + int bt, bu = aa(-1);
> + double bv, bw, *by, *bz, **ca;
> + __attribute__((__vector_size__(8 * sizeof(double)))) double cb;
> + cb[4] = 1.75;
> + cb[5] = -8.0;
> + cb[7] = 1.75;
> + ca = &by;
> + bt = bu;
> + bv = -65536;
> + bz = &bw;
> + cb[2] = 0 + (bm ? -8.0 : bg);
> + if (-bl >= 0) {
> + if (bm)
> + goto bx;
> + ca = &bz;
> + }
> + cb[3] = 524420 + bt;
> + az = bl - (1646 ^ bl);
> + bw = 2.0 * bv;
> + bv = 2.0 * bw;
> + *ca = &bv;
> +bx:
> + cb[1] = -4.0 + 2.5 * bv;
> + bc = 267503909 - bn;
> + bi = *ca;
> + *by = 131072 - 5.0 * bw;
> + *bz = 3.0 * bg + 0.375 * bv;
> + ay = ba = bl + 15 * az;
> + bj = -__builtin_popcount((unsigned char)(109 + az));
> + an = 0;
> +
> + an = a(an, cb[1]);
> + an = a(an, cb[2]);
> + an = a(an, cb[3]);
> + an = a(an, cb[4]);
> + an = a(an, cb[5]);
> + an = a(an, cb[7]);
> +
> + an = a(an, bw);
> + an = an^(int)bg;
> + return an;
> +}
> +int main() {
> + am = s(1, 1, 1, 1, 3, 1, 1);
> + al = am;
> + if (al != 0xfff97f7f)
> + __builtin_abort ();
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/sse2-init-v2df-2.c
> b/gcc/testsuite/gcc.target/i386/sse2-init-v2df-2.c
> index 3f58098af8d..0b4fbd4c4fe 100644
> --- a/gcc/testsuite/gcc.target/i386/sse2-init-v2df-2.c
> +++ b/gcc/testsuite/gcc.target/i386/sse2-init-v2df-2.c
> @@ -9,15 +9,17 @@ v2df fx0(double x) { return (v2df){ x, 0.0 }; }
> v2df f0x(double x) { return (v2df){ 0.0, x }; }
> v2df fxx(double x) { return (v2df){ x, x }; }
> v2df fxy(double x, double y) { return (v2df){ x, y }; }
> +v2df fyx(double x, double y) { return (v2df){ y, x }; }
>
> v2df fm0() { return (v2df){ m, 0.0 }; }
> v2df f0m() { return (v2df){ 0.0, m }; }
> v2df fmm() { return (v2df){ m, m }; }
> v2df fmn() { return (v2df){ m, n }; }
>
> -/* { dg-final { scan-assembler-times "movq" 5 } } */
> +/* { dg-final { scan-assembler-times "movq" 4 } } */
> /* { dg-final { scan-assembler-times "pslldq" 2 } } */
> /* { dg-final { scan-assembler-times "unpcklpd" 2 } } */
> -/* { dg-final { scan-assembler-times "movsd" 3 } } */
> +/* { dg-final { scan-assembler-times "movsd" 4 } } */
> /* { dg-final { scan-assembler-times "movhpd" 2 } } */
> +/* { dg-final { scan-assembler-times "movupd" 1 } } */
>
> diff --git a/gcc/testsuite/gcc.target/i386/sse4_1-init-v2df-2.c
> b/gcc/testsuite/gcc.target/i386/sse4_1-init-v2df-2.c
> index 218cc8bf896..06dc240a665 100644
> --- a/gcc/testsuite/gcc.target/i386/sse4_1-init-v2df-2.c
> +++ b/gcc/testsuite/gcc.target/i386/sse4_1-init-v2df-2.c
> @@ -9,15 +9,17 @@ v2df fx0(double x) { return (v2df){ x, 0.0 }; }
> v2df f0x(double x) { return (v2df){ 0.0, x }; }
> v2df fxx(double x) { return (v2df){ x, x }; }
> v2df fxy(double x, double y) { return (v2df){ x, y }; }
> +v2df fyx(double x, double y) { return (v2df){ y, x }; }
>
> v2df fm0() { return (v2df){ m, 0.0 }; }
> v2df f0m() { return (v2df){ 0.0, m }; }
> v2df fmm() { return (v2df){ m, m }; }
> v2df fmn() { return (v2df){ m, n }; }
>
> -/* { dg-final { scan-assembler-times "movq" 5 } } */
> +/* { dg-final { scan-assembler-times "movq" 4 } } */
> /* { dg-final { scan-assembler-times "pslldq" 2 } } */
> /* { dg-final { scan-assembler-times "movddup" 2 } } */
> -/* { dg-final { scan-assembler-times "movsd" 1 } } */
> +/* { dg-final { scan-assembler-times "movsd" 2 } } */
> /* { dg-final { scan-assembler-times "movhpd" 2 } } */
> +/* { dg-final { scan-assembler-times "movupd" 1 } } */
>
> --
> 2.51.0
