[Bug target/113059] New: [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[Bug target/113059] New: [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

            Bug ID: 113059
           Summary: [14 regression] fftw fails tests for -O3 -m32
                    -march=znver2 since r14-6210-ge44ed92dbbe9d4
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sjames at gcc dot gnu.org
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

fftw started failing tests (very inconsistently - almost always fails, but
sometimes w/ segfault, sometimes with wrong results) since
r14-6210-ge44ed92dbbe9d4.

Might be a continuation of PR112572. We had pinskia's workaround patch applied
until r14-6210-ge44ed92dbbe9d4 landed so I may need to double check.

I can reproduce with the following:
```
./configure CFLAGS="-O3 -m32 -march=znver2 -ggdb3"
make -j$(nproc)
make -j$(nproc) check
```

Just `tests/bench --verbose=1 --verify //obrd896` seems to be a good crasher
for me.

```
Program received signal SIGSEGV, Segmentation fault.
r2cf_32 (R0=0xffff97c0, R1=0xffff98d0, Cr=0xffff97c0, Ci=<optimized out>,
rs=<optimized out>, csr=0x56690040, csi=0x0, v=<optimized out>, ivs=1, ovs=1)
at r2cf_32.c:490
490                         Ci[WS(csi, 8)] = T29 - T28;
(gdb) bt
#0  r2cf_32 (R0=0xffff97c0, R1=0xffff98d0, Cr=0xffff97c0, Ci=<optimized out>,
rs=<optimized out>, csr=0x56690040, csi=0x0, v=<optimized out>, ivs=1, ovs=1)
at r2cf_32.c:490
#1  0x56632963 in dobatch_r2hc (ego=0x566916b0, I=0x56680e80, O=0x56680e80,
buf=0xffff97c0, batchsz=1) at direct-r2c.c:91
#2  0x56632a5d in iterate (ego=0x566916b0, I=<optimized out>, O=<optimized
out>, dobatch=0x56632880 <dobatch_r2hc>) at direct-r2c.c:142
#3  0x565a09f5 in fftw_rdft_solve (ego_=0x566916b0, p_=0x566921d0) at
solve.c:29
#4  0x565603eb in measure (iter=1, p=<optimized out>, pln=<optimized out>) at
timer.c:136
#5  fftw_measure_execution_time (plnr=0x56684690, pln=0x566916b0, p=0x566921d0)
at timer.c:159
#6  0x5655d853 in evaluate_plan (ego=ego@entry=0x56684690,
pln=pln@entry=0x566916b0, p=p@entry=0x566921d0) at planner.c:460
#7  0x5655e773 in search0 (ego=ego@entry=0x56684690, p=p@entry=0x566921d0,
slvndx=slvndx@entry=0xffffbbac, flagsp=<optimized out>) at planner.c:529
#8  0x5655eb37 in search (flagsp=0xffffbbb0, slvndx=0xffffbbac, p=0x566921d0,
ego=0x56684690) at planner.c:600
#9  mkplan (ego=<optimized out>, p=<optimized out>) at planner.c:711
[...]
```

Reducing is challenging so far, as FFTW is massive. I'm not going to waste much
more time on that, but I'll do the usual bisecting object files and such now.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |i?86-*-*
   Target Milestone|---                         |14.0

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Any progress with the bisection?
Or at least details what exactly are you compiling (with what patches etc.)?

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #2 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #1)
> Any progress with the bisection?

sorry, not yet. I've been away from the computer mostly for an emergency.

I did make a start, but I got frustrated with how the Makefile deps seem broken
(replacing a .o and running 'make' doesn't trigger a rebuild correctly in all
cases).

> Or at least details what exactly are you compiling (with what patches etc.)?

I can reproduce it consistently with:
```
wget https://www.fftw.org/fftw-3.3.10.tar.gz
tar xvf fftw-3.3.10.tar.xz && cd fftw-3.3.10
./configure CFLAGS="-O3 -m32 -march=znver2 -ggdb3"
make -j$(nproc)
make -j$(nproc) check
```

Let me know if you need more.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Seems the package's configure is affected by most likely the modern C changes,
I see
--- config.h.good       2023-12-22 17:47:44.615207332 +0100
+++ config.h.bad        2023-12-22 17:46:42.304068624 +0100
@@ -37,7 +37,7 @@
 /* #undef F77_FUNC_ */

 /* Define if F77_FUNC and F77_FUNC_ are equivalent. */
-/* #undef F77_FUNC_EQUIV */
+#define F77_FUNC_EQUIV 1

 /* Define if F77 and FC dummy `main' functions are identical. */
 /* #undef FC_DUMMY_MAIN_EQ_F77 */
@@ -404,7 +404,7 @@

 /* Include g77-compatible wrappers in addition to any other Fortran wrappers.
    */
-/* #undef WITH_G77_WRAPPERS */
+#define WITH_G77_WRAPPERS 1

 /* Use our own aligned malloc routine; mainly helpful for Windows systems
    lacking aligned allocation system-library routines. */
diff in config.h between my system gcc 12 and gcc 14 snapshot.
But that isn't the reason for the failure.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Bisection points to hc2cf2_16.o.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
$ /opt/notnfs/gcc-bisect/obj/gcc/cc1.r14-6209 -quiet -nostdinc -O3 -m32
-march=znver2 -ggdb3 hc2cf2_16.i -o hc2cf2_16.s1
$ /opt/notnfs/gcc-bisect/obj/gcc/cc1.r14-6210 -quiet -nostdinc -O3 -m32
-march=znver2 -ggdb3 hc2cf2_16.i -o hc2cf2_16.s2
$ gcc -c -o /tmp/hc2cf2_16.o -xassembler /tmp/hc2cf2_16.s1 -m32
$ gcc -O3 -m32 -march=znver2 -ggdb3 -o bench2 bench-bench.o bench-hook.o
bench-fftw-bench.o /tmp/hc2cf2_16.o ../.libs/libfftw3.a
../libbench2/libbench2.a -lm; ./bench2 --verbose=1   --verify 'obc3x13'
--verify 'ibc3x13' --verify '//ifr4752'
obc3x13 2.86313e-16 2.84445e-16 7.6393e-16
ibc3x13 2.55967e-16 2.84445e-16 7.42983e-16
//ifr4752 2.74708e-16 1.85534e-15 9.24976e-16
$ gcc -c -o /tmp/hc2cf2_16.o -xassembler /tmp/hc2cf2_16.s2 -m32
$ gcc -O3 -m32 -march=znver2 -ggdb3 -o bench2 bench-bench.o bench-hook.o
bench-fftw-bench.o /tmp/hc2cf2_16.o ../.libs/libfftw3.a
../libbench2/libbench2.a -lm; ./bench2 --verbose=1   --verify 'obc3x13'
--verify 'ibc3x13' --verify '//ifr4752'
obc3x13 2.86313e-16 2.84445e-16 7.6393e-16
ibc3x13 2.55967e-16 2.84445e-16 7.42983e-16
corrupted size vs. prev_size
Aborted (core dumped)

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 56923
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56923&action=edit
hc2cf2_16.i.xz

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
With -g0 in addition the assembly difference is
--- hc2cf2_16.s1        2023-12-22 13:14:14.000000000 -0500
+++ hc2cf2_16.s2        2023-12-22 13:14:06.000000000 -0500
@@ -16,7 +16,6 @@ hc2cf2_16:
 .LCFI4:
        movl    552(%esp), %eax
        movl    528(%esp), %edx
-       movl    540(%esp), %esi
        movl    556(%esp), %ebp
        decl    %eax
        sall    $6, %eax

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

Florian Weimer <fw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fw at gcc dot gnu.org

--- Comment #8 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> Seems the package's configure is affected by most likely the modern C
> changes,
> I see
> --- config.h.good	2023-12-22 17:47:44.615207332 +0100
> +++ config.h.bad	2023-12-22 17:46:42.304068624 +0100
> @@ -37,7 +37,7 @@
>  /* #undef F77_FUNC_ */
>  
>  /* Define if F77_FUNC and F77_FUNC_ are equivalent. */
> -/* #undef F77_FUNC_EQUIV */
> +#define F77_FUNC_EQUIV 1
>  
>  /* Define if F77 and FC dummy `main' functions are identical. */
>  /* #undef FC_DUMMY_MAIN_EQ_F77 */
> @@ -404,7 +404,7 @@
>  
>  /* Include g77-compatible wrappers in addition to any other Fortran
> wrappers.
>     */
> -/* #undef WITH_G77_WRAPPERS */
> +#define WITH_G77_WRAPPERS 1
>  
>  /* Use our own aligned malloc routine; mainly helpful for Windows systems
>     lacking aligned allocation system-library routines. */
> diff in config.h between my system gcc 12 and gcc 14 snapshot.
> But that isn't the reason for the failure.

I don't see a different to system gcc 13. It has F77_FUNC_EQUIV and
WITH_G77_WRAPPERS set as well. Do you see this for all build variants of fftw?

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Seems in *.reload we have:
(insn 5 4 6 2 (set (reg/v/f:SI 4 si [orig:504 Im ] [504])
        (mem/f/c:SI (plus:SI (reg/f:SI 7 sp)
                (const_int 540 [0x21c])) [3 Im+0 S4 A32])) "hc2cf2_16.c":456:1
85 {*movsi_internal}
     (expr_list:REG_EQUIV (mem/f/c:SI (plus:SI (reg/f:SI 16 argp)
                (const_int 12 [0xc])) [3 Im+0 S4 A32])
        (nil)))
...
(insn 487 486 488 3 (set (mem/f/c:SI (plus:SI (reg/f:SI 7 sp)
                (const_int 540 [0x21c])) [3 Im+0 S4 A32])
        (reg/v/f:SI 4 si [orig:504 Im ] [504])) "hc2cf2_16.c":691:14 85
{*movsi_internal}
     (nil))
(insn 488 487 489 3 (set (reg/v/f:SI 4 si [orig:500 W ] [500])
        (reg/v/f:SI 0 ax [orig:500 W ] [500])) "hc2cf2_16.c":691:14 85
{*movsi_internal}
     (nil))
...
(insn 491 490 474 3 (set (reg/v/f:SI 2 cx [orig:504 Im ] [504])
        (mem/f/c:SI (plus:SI (reg/f:SI 7 sp)
                (const_int 540 [0x21c])) [3 Im+0 S4 A32])) "hc2cf2_16.c":691:14
85 {*movsi_internal}
     (nil))
Now, postreload removes the useless store with uid 487 and due to the swapping
of vzeroupper pass with postreload, the good vs. bad difference before gcse is
 (insn 5 4 6 2 (set (reg/v/f:SI 4 si [orig:504 Im ] [504])
         (mem/f/c:SI (plus:SI (reg/f:SI 7 sp)
                 (const_int 540 [0x21c])) [3 Im+0 S4 A32])) "hc2cf2_16.c":456:1
85 {*movsi_internal}
-     (expr_list:REG_EQUIV (mem/f/c:SI (plus:SI (reg/f:SI 16 argp)
-                (const_int 12 [0xc])) [3 Im+0 S4 A32])
-        (nil)))
+     (expr_list:REG_UNUSED (reg/v/f:SI 4 si [orig:504 Im ] [504])
+        (expr_list:REG_EQUIV (mem/f/c:SI (plus:SI (reg/f:SI 16 argp)
+                    (const_int 12 [0xc])) [3 Im+0 S4 A32])
+            (nil))))
i.e. an extra REG_UNUSED note (on some others as well).
At that point the note is correct.
Then comes gcse and sets cx to si at the start of the bb where insn 491 used to
appear, extending the lifetime of the register, but the REG_UNUSED note is not
removed.
And then comes the pro_and_epilogue pass and because of the REG_UNUSED note
deletes the load in insn 5, even when it is now actually used.
So we are back to the PR112572 discussions.
Whether make REG_UNUSED notes only trusted in passes which explicitly request
computation of the notes problem as Richard proposed, or whether just have a
couple of passes known to screw up REG_UNUSED notes to mark them as
non-trustworthy (mainly cse/gcse/postreload-cse), or just when those passes
actually extend lifetime of something, or attempt to remove REG_UNUSED notes
actively when they extend stuff.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #8)
> (In reply to Jakub Jelinek from comment #3)
> > Seems the package's configure is affected by most likely the modern C
> > changes,
> > I see
> > --- config.h.good	2023-12-22 17:47:44.615207332 +0100
> > +++ config.h.bad	2023-12-22 17:46:42.304068624 +0100
> > @@ -37,7 +37,7 @@
> >  /* #undef F77_FUNC_ */
> >  
> >  /* Define if F77_FUNC and F77_FUNC_ are equivalent. */
> > -/* #undef F77_FUNC_EQUIV */
> > +#define F77_FUNC_EQUIV 1
> >  
> >  /* Define if F77 and FC dummy `main' functions are identical. */
> >  /* #undef FC_DUMMY_MAIN_EQ_F77 */
> > @@ -404,7 +404,7 @@
> >  
> >  /* Include g77-compatible wrappers in addition to any other Fortran
> > wrappers.
> >     */
> > -/* #undef WITH_G77_WRAPPERS */
> > +#define WITH_G77_WRAPPERS 1
> >  
> >  /* Use our own aligned malloc routine; mainly helpful for Windows systems
> >     lacking aligned allocation system-library routines. */
> > diff in config.h between my system gcc 12 and gcc 14 snapshot.
> > But that isn't the reason for the failure.
> 
> I don't see a different to system gcc 13. It has F77_FUNC_EQUIV and
> WITH_G77_WRAPPERS set as well. Do you see this for all build variants of
> fftw?

Ah, seems I don't have system gfortran installed, which is probably the cause
of the config.h difference.  Sorry for the false alarm.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
IMHO REG_UNUSED notes should stay correct or be removed.  Instead of a full
solution can we just wipe them in postreload-cse as we know that pass will
break them?  Any user then should recompute the notes.  Originally pass
properties were thought of a vehicle indicating these kind of IL features, so
we
could add PROP_rtl_unused_notes and if not set treat them as possibly
invalid when present (instead of actively wiping them), and when a pass
requires them re-compute them.

The problem with properties is that they are always set/unset irrespective
of whether a pass did something destroying it.  That could be
circumvented by postreload-cse only clearing the flag when it does
something.

Not (re-)using pass properties for this but a flag in struct function
works as well of course (or treat it as part of the DF state).

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #11)
> IMHO REG_UNUSED notes should stay correct or be removed.  Instead of a full
> solution can we just wipe them in postreload-cse as we know that pass will
> break them?  Any user then should recompute the notes.  Originally pass
> properties were thought of a vehicle indicating these kind of IL features,
> so we
> could add PROP_rtl_unused_notes and if not set treat them as possibly
> invalid when present (instead of actively wiping them), and when a pass
> requires them re-compute them.
> 
> The problem with properties is that they are always set/unset irrespective
> of whether a pass did something destroying it.  That could be
> circumvented by postreload-cse only clearing the flag when it does
> something.
> 
> Not (re-)using pass properties for this but a flag in struct function
> works as well of course (or treat it as part of the DF state).

I was hoping we could have some df checking which would discover invalid
REG_UNUSED notes and so we would know which passes need tweaking, but I'm
afraid my df knowledge is insufficient for that.
Dunno how often postreload-cse actually extends life time of some register,
which should determine whether we want to drop REG_UNUSED notes unconditionally
at the end of postreload-cse, or whether we e.g. want just some flag whether
we've extended lifetime of something during it and only remove REG_UNUSED notes
if that flag is set.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #13 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 10 Jan 2024, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059
> 
> --- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #11)
> > IMHO REG_UNUSED notes should stay correct or be removed.  Instead of a full
> > solution can we just wipe them in postreload-cse as we know that pass will
> > break them?  Any user then should recompute the notes.  Originally pass
> > properties were thought of a vehicle indicating these kind of IL features,
> > so we
> > could add PROP_rtl_unused_notes and if not set treat them as possibly
> > invalid when present (instead of actively wiping them), and when a pass
> > requires them re-compute them.
> > 
> > The problem with properties is that they are always set/unset irrespective
> > of whether a pass did something destroying it.  That could be
> > circumvented by postreload-cse only clearing the flag when it does
> > something.
> > 
> > Not (re-)using pass properties for this but a flag in struct function
> > works as well of course (or treat it as part of the DF state).
> 
> I was hoping we could have some df checking which would discover invalid
> REG_UNUSED notes and so we would know which passes need tweaking, but I'm
> afraid my df knowledge is insufficient for that.
> Dunno how often postreload-cse actually extends life time of some register,
> which should determine whether we want to drop REG_UNUSED notes unconditionally
> at the end of postreload-cse, or whether we e.g. want just some flag whether
> we've extended lifetime of something during it and only remove REG_UNUSED notes
> if that flag is set.

In theory the insn analysis could look at REG_UNUSED notes and mark
them as "to be extended" (eventually even pointing to the note).  When
eliminating to a such marked REG we could either remove the note or
scrap all notes.

That said - not sure how exactly postreload-cse keeps its state.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note for GCC 15, I think it might be best if single_set moves away from
REG_UNUSED to use df directly, like Steve provided a similar function in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40209#c6 . That will fix most but
not all of the issue.
I am almost positive insn split will have issues too.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #15 from Sam James <sjames at gcc dot gnu.org> ---
at what point can we consider a revert for 14 and revisit for 15? there's a few
things which keep cropping up and I don't want to be ignoring other regressions
because I'm assuming it's the same as this

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The question is revert what exactly?
If we revert r14-6210, we get back the other P1.  Or do you mean revert
r14-5355?
I guess another option is move the vzeroupper pass one pass later, i.e. after
pass_gcse.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #16)
> The question is revert what exactly?
> If we revert r14-6210, we get back the other P1.  Or do you mean revert
> r14-5355?
> I guess another option is move the vzeroupper pass one pass later, i.e.
> after pass_gcse.

I think moving mdreorg passes as late as possible esp. when they don't play
well with DF/notes is a good thing.  Maybe even after pass_rtl_dse2 and
thus after shrink-wrapping?

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #17)
> (In reply to Jakub Jelinek from comment #16)
> > The question is revert what exactly?
> > If we revert r14-6210, we get back the other P1.  Or do you mean revert
> > r14-5355?
> > I guess another option is move the vzeroupper pass one pass later, i.e.
> > after pass_gcse.
> 
> I think moving mdreorg passes as late as possible esp. when they don't play
> well with DF/notes is a good thing.  Maybe even after pass_rtl_dse2 and
> thus after shrink-wrapping?

The thing is that the vzeroupper pass actually plays well with DF notes, the
problem is that it now (in GCC 14) asks for them to be computed.
The first issue was that vzeroupper before postreload_cse computed the notes,
then
postreload_cse CSEd something and made the REG_UNUSED invalid without killing
them and then later passes went wrong because of the incorrect notes.
This issue is that vzeroupper now after postreload_cse but before gcse2
computes notes, then gcse2 CSEs something and makes REG_UNUSED invalid, rest is
the same.
But, I believe gcse2 is the last CSE-ish pass.
I wouldn't move it too much further, because I don't remember the interactions
between vzeroupper, splitting and peepholes.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 57258
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57258&action=edit
gcc14-pr113059.patch

So in patch form like this.  Untested so far.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #20 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 30 Jan 2024, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059
> 
> --- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #17)
> > (In reply to Jakub Jelinek from comment #16)
> > > The question is revert what exactly?
> > > If we revert r14-6210, we get back the other P1.  Or do you mean revert
> > > r14-5355?
> > > I guess another option is move the vzeroupper pass one pass later, i.e.
> > > after pass_gcse.
> > 
> > I think moving mdreorg passes as late as possible esp. when they don't play
> > well with DF/notes is a good thing.  Maybe even after pass_rtl_dse2 and
> > thus after shrink-wrapping?
> 
> The thing is that the vzeroupper pass actually plays well with DF notes, the
> problem is that it now (in GCC 14) asks for them to be computed.
> The first issue was that vzeroupper before postreload_cse computed the notes,
> then
> postreload_cse CSEd something and made the REG_UNUSED invalid without killing
> them and then later passes went wrong because of the incorrect notes.
> This issue is that vzeroupper now after postreload_cse but before gcse2
> computes notes, then gcse2 CSEs something and makes REG_UNUSED invalid, rest is
> the same.
> But, I believe gcse2 is the last CSE-ish pass.
> I wouldn't move it too much further, because I don't remember the interactions
> between vzeroupper, splitting and peepholes.

OK, so the "real" revert would then simply kill the notes actively
again after vzeroupper?  Btw, DSE also uses cselib, but I'm not sure
whether it uses REG_UNUSED but IIRC it does "redundant store" removal
and it does replace reads in some cases ... (not sure if after reload
though).

So for maximum safety if we'd have a way to kill off REG_UNUSED maybe
we should do that instead?  OTOH any "stray" valid REG_UNUSED
notes not causing issues with gcse or postreload_cse might not be
preserved and cause missed optimizations later ...

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #19)
> Created attachment 57258 [details]
> gcc14-pr113059.patch
> 
> So in patch form like this.  Untested so far.

LGTM.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, I have quickly looked at REG_UNUSED notes on insn-recog.cc (as a randomly
picked large GCC object).  Ignoring REG_UNUSED notes for flags register (which
are extremely common in lots of passes), seems we have just a few such notes
before RA and probably combiner kills them?
insn-recog.cc.292r.cse2:        (expr_list:REG_UNUSED (reg:SI 207)
insn-recog.cc.293r.dse1:        (expr_list:REG_UNUSED (reg:SI 207)
insn-recog.cc.294r.fwprop2:        (expr_list:REG_UNUSED (reg:SI 207)
insn-recog.cc.296r.init-regs:        (expr_list:REG_UNUSED (reg:SI 207)
insn-recog.cc.297r.ud_dce:        (expr_list:REG_UNUSED (reg:SI 207)
insn-recog.cc.298r.combine:      REG_UNUSED r207:SI
insn-recog.cc.298r.combine:      REG_UNUSED r207:SI
Then I see tons of REG_UNUSED notes for non-flags in IRA dump (all new), but
they don't appear after it, so most likely LRA removes them.  Even the flags
related REG_UNUSED don't appear in *.reload dump (except some details
comments), nor *.postreload etc.
And (note, this isn't a build with -mavx*, so no vzeroupper), they reappear
only in dse2 dump and keep appearing from that point onwards.
Similarly REG_DEAD notes don't appear in the IL between *.reload and
*.pro_and_epilogue inclusive.
Seems LRA does this in update_inc_notes, reload did that in reload function.

So, maybe safer than the above patch would be simply do that too in vzeroupper
pass (I guess we don't need to update the REG_INC notes).
Let me write another patch.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 57259
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57259&action=edit
gcc14-pr113059-2.patch

Untested patch to remove the notes instead of moving the pass around further.

[Bug target/113059] [14 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #24 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:d413df070ba5efadd2fb8b6c6aa6003b8cae617b

commit r14-8797-gd413df070ba5efadd2fb8b6c6aa6003b8cae617b
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Feb 5 09:33:26 2024 +0100

    i386: Clear REG_UNUSED and REG_DEAD notes from the IL at the end of
vzeroupper pass [PR113059]

    The move of the vzeroupper pass from after reload pass to after
    postreload_cse helped only partially, CSE-like passes can still invalidate
    those notes (especially REG_UNUSED) if they use some earlier register
    holding some value later on in the IL.

    So, either we could try to move it one pass further after gcse2 and hope
    no later pass invalidates the notes, or the following patch attempts to
    restore the REG_DEAD/REG_UNUSED state from GCC 13 and earlier, where
    the LRA or reload passes remove all REG_DEAD/REG_UNUSED notes and the notes
    reappear only at the start of dse2 pass when it calls
      df_note_add_problem ();
      df_analyze ();
    So, effectively
              NEXT_PASS (pass_postreload_cse);
              NEXT_PASS (pass_gcse2);
              NEXT_PASS (pass_split_after_reload);
              NEXT_PASS (pass_ree);
              NEXT_PASS (pass_compare_elim_after_reload);
              NEXT_PASS (pass_thread_prologue_and_epilogue);
    passes operate without those notes in the IL.
    While in GCC 14 mode switching computes the notes problem at the start of
    vzeroupper, the patch below removes them at the end of the pass again, so
    that the above passes continue to operate without them.

    2024-02-05  Jakub Jelinek  <jakub@redhat.com>

            PR target/113059
            * config/i386/i386-features.cc (rest_of_handle_insert_vzeroupper):
            Remove REG_DEAD/REG_UNUSED notes at the end of the pass before
            df_analyze call.

[Bug target/113059] [15 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[14 regression] fftw fails  |[15 regression] fftw fails
                   |tests for -O3 -m32          |tests for -O3 -m32
                   |-march=znver2 since         |-march=znver2 since
                   |r14-6210-ge44ed92dbbe9d4    |r14-6210-ge44ed92dbbe9d4
   Last reconfirmed|                            |2024-02-05
   Target Milestone|14.0                        |15.0
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Worked around for GCC 14, but we should fix this for real for GCC 15.

[Bug target/113059] [15 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #26 from Sam James <sjames at gcc dot gnu.org> ---
*** Bug 114079 has been marked as a duplicate of this bug. ***

[Bug target/113059] [15 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #27 from Sam James <sjames at gcc dot gnu.org> ---
Can someone sanity-check me on this by trying the instructions at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114079#c0?

I think I can still hit the original crash, it's just flaky (it might pass
sometimes).

[Bug target/113059] [15 regression] fftw fails tests for -O3 -m32 -march=znver2 since r14-6210-ge44ed92dbbe9d4

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113059

--- Comment #28 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Sam James from comment #27)
> Can someone sanity-check me on this by trying the instructions at
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114079#c0?
> 
> I think I can still hit the original crash, it's just flaky (it might pass
> sometimes).

I suspect the fix for PR114190 solves this.