[Bug c/116454] New: Under certain conditions, gcc produces an extra "addq $8, 40(%rsp)" instruction overfreeing the stack.

[Bug c/116454] New: Under certain conditions, gcc produces an extra "addq $8, 40(%rsp)" instruction overfreeing the stack.

[Liam_Kerrigan at outlook dot com]

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

            Bug ID: 116454
           Summary: Under certain conditions, gcc produces an extra "addq
                    $8, 40(%rsp)" instruction overfreeing the stack.
           Product: gcc
           Version: 14.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Liam_Kerrigan at outlook dot com
  Target Milestone: ---

Created attachment 58973
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58973&action=edit
Compiling main.c as described above produces an executable that returns 1, but
should return 0.

gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=C:/w64devkit/bin/../libexec/gcc/x86_64-w64-mingw32/14.2.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: /gcc-14.2.0/configure --prefix=/w64devkit
--with-sysroot=/w64devkit/x86_64-w64-mingw32
--with-native-system-header-dir=/include --target=x86_64-w64-mingw32
--host=x86_64-w64-mingw32 --enable-static --disable-shared --with-pic
--with-gmp-include=/deps/include --with-gmp-lib=/deps/lib
--with-mpc-include=/deps/include --with-mpc-lib=/deps/lib
--with-mpfr-include=/deps/include --with-mpfr-lib=/deps/lib
--enable-languages=c,c++,fortran --enable-libgomp --enable-threads=posix
--enable-version-specific-runtime-libs --disable-dependency-tracking
--disable-lto --disable-multilib --disable-nls --disable-win32-registry
--enable-mingw-wildcard CFLAGS_FOR_TARGET=-Os CXXFLAGS_FOR_TARGET=-Os
LDFLAGS_FOR_TARGET=-s CFLAGS=-Os CXXFLAGS=-Os LDFLAGS=-s
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.2.0 (GCC)

The complete command line that triggers the bug:
gcc -ffast-math main.c -o e.exe

Potentially helpful notes:
The bug does not occur if the line "*++ptr * 1j;" is replaced with "++ptr; *ptr
* 1j;" in main.c
If that line is replaced as such, the only difference in the assembly generated
by gcc with the -S is that one of the two consecutive "addq $8, 40(%rsp)"
instructions is removed.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
            Summary|`*++ptr * 1j` is handled    |[12/13/14/15 Regression]
                   |incorrectly with            |`*++ptr * 1j` is handled
                   |-ffast-math, increments ptr |incorrectly with
                   |twice                       |-ffast-math, increments ptr
                   |                            |twice
      Known to work|                            |3.4.6
   Target Milestone|---                         |12.5
      Known to fail|                            |12.1.0

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
      Known to work|                            |4.2.1
   Last reconfirmed|                            |2024-08-21
      Known to fail|                            |4.3.0
             Status|UNCONFIRMED                 |NEW
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=30172

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Introduced by r0-77972-g9f5396713d9e18 . Missing a save_expr before using arg0
with IMAGPART_EXPR/IMAGPART_EXPR when handling `z * +-I`.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I will take care of fixing this.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[Liam_Kerrigan at outlook dot com]

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

--- Comment #3 from Liam Kerrigan <Liam_Kerrigan at outlook dot com> ---
(In reply to Andrew Pinski from comment #2)
> I will take care of fixing this.

Thank you for fixing my original title, which I now realize was really
incorrect. It is very impressive to me how quickly you are finding and solving
this issue!

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Another testcase:
```
static int t = 0;
_Complex float f()
{
        t++;
        return 0;
}
int main() {
        t = 0;
        f() * 1j; // incorrectly invokes f() twice
        if (t != 1)
          __builtin_abort();
}
```

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 58974
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58974&action=edit
Patch which I am testing

Simple tests on the testcase works and we get the correct behavior now.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> Created attachment 58974 [details]
> Patch which I am testing
> 
> Simple tests on the testcase works and we get the correct behavior now.

This patch causes some testcase regressions. Looking into them now.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|pinskia at gcc dot gnu.org         |unassigned at gcc dot gnu.org
             Status|ASSIGNED                    |NEW

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So the failure are:

+FAIL: gcc.dg/builtins-20.c (test for excess errors)
+FAIL: gcc.dg/builtins-54.c (test for excess errors)
+FAIL: gcc.dg/fold-complex-1.c scan-tree-dump-times original "COMPLEX_EXPR" 0
+FAIL: gcc.dg/pr30172-1.c scan-tree-dump gimple " = -x;\\n.*COMPLEX_EXPR <y, "
+FAIL: gcc.dg/pr30172-1.c scan-tree-dump gimple " = -y;\\n.*COMPLEX_EXPR
<[^,]*, x>"
+FAIL: gcc.dg/pr30172-1.c scan-tree-dump gimple "COMPLEX_EXPR <1.0e\\\\+0, x>"
+FAIL: gcc.dg/pr30172-1.c scan-tree-dump gimple "COMPLEX_EXPR <x, y>"

This is due to not optimizing some stuff later on.

My fix is "correct", just we lose some optimizations, I am not going to work on
this any more at this point. Someone else will need to fix this maybe a better
way.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice since r0-77972-g9f5396713d9e18

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine then.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice since r0-77972-g9f5396713d9e18

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
One of the issues is that

REALPART_EXPR <SAVE_EXPR <COMPLEX_EXPR <IMAGPART_EXPR <x>, 0.0>>

is no longer simplified.  I don't think __real folding can look through
a SAVE_EXPR but instead of building SAVE_EXPR <COMPLEX_EXPR <...>> we
could build COMPLEX_EXPR <IMAGPART_EXPR <SAVE_EXPR <x>>, 0.0> here.

I'm not exactly sure why we consider a function parameter (like in
gcc.dg/fold-complex-1.c) as requiring a SAVE_EXPR, we're only resorting to
tree_invariant_p_1 instead of looking at possible side-effects.

For builtins-20.c we FAIL

  if (hypot (tan(-x), tan(-fabs(y))) != hypot (tan(x), tan(y)))
    link_error ();

  if (ccos(tan(x)+sin(x-y)*1i) != ccos(-tan(-x)-sin(y-x)*1i))
    link_error ();

  if (tan(x)+tan(y)*1i != -~(tan(-x)+tan(y)*1i))
    link_error ();

  if (hypotf (tanf(-x), tanf(-fabsf(y))) != hypotf (tanf(x), tanf(y)))
    link_error ();

  if (ccosf(tanf(x)+sinf(x-y)*1i) != ccosf(-tanf(-x)-sinf(y-x)*1i))
    link_error ();

  if (tanf(x)+tanf(y)*1i != -~(tanf(-x)+tanf(y)*1i))
    link_error ();

  if (hypotl (tanl(-x), tanl(-fabsl(y))) != hypotl (tanl(x), tanl(y)))
    link_error ();

  if (ccosl(tanl(x)+sinl(x-y)*1i) != ccosl(-tanl(-x)-sinl(y-x)*1i))
    link_error ();

  if (tanl(x)+tanl(y)*1i != -~(tanl(-x)+tanl(y)*1i))
    link_error ();

for the first we have (in .optimized):

  <bb 2> [local count: 1073741824]:
  _1 = -x_96(D);
  y_115 = y_97(D) * 3.6e+1;
  _32 = tan (y_115);
  _33 = tan (_1);
  _34 = hypot (_33, _32);
  _36 = tan (x_96(D));
  _37 = hypot (_36, _32);
  if (_34 != _37)

in isolation we still optimize this so it's an odd interaction with
surrounding code.  We do observe similar differences with respect
to SAVE_EXPRs compared to GCC 14.

There's a difference after backprop:

   <bb 2> [local count: 1073741824]:
-  _1 = -x_64(D);
+  _1 = -x_96(D);

   <bb 37> [local count: 1073741824]:
-  _32 = tan (y_83);
-  _33 = tan (x_64(D));
+  _32 = tan (y_115);
+  _33 = tan (_1);
   _34 = hypot (_33, _32);
-  _35 = tan (y_83);
-  _36 = tan (x_64(D));
+  _35 = tan (y_115);
+  _36 = tan (x_96(D));
   _37 = hypot (_36, _35);
   if (_34 != _37)

where GCC 14 does

   <bb 37> [local count: 1073741824]:
-  _31 = .COPYSIGN (y_83, -1.0e+0);
-  _32 = tan (_31);
-  _33 = tan (_1);
+  _32 = tan (y_83);
+  _33 = tan (x_64(D));

but patched trunk only

   <bb 37> [local count: 1073741824]:
-  _31 = .COPYSIGN (y_115, -1.0e+0);
-  _32 = tan (_31);
+  _32 = tan (y_115);
   _33 = tan (_1);

and that's because on trunk there's another use of _33 not ignoring sign
which is already folded on the branch:

  <bb 51> [local count: 1073741824]:
  _65 = COMPLEX_EXPR <-0.0, _35>;
  _66 = _52 + _65;
  _67 = COMPLEX_EXPR <_33, 0.0>;
  _68 = _65 + _67;
  _69 = [conj_expr] _68;
  _70 = -_69;
  if (_66 != _70)

that's the

  if (tan(x)+tan(y)*1i != -~(tan(-x)+tan(y)*1i))
    link_error ();

test which only complex lowering can now turn into something optimized,
but we don't get any backprop after that.

There's a missed optimization in backprop in that it doesn't realize
we already compute tan(x) so instead of changing tan(-x) to compute
tan(x) it could have used the result of the existing tan(x) compute.

That's also something VN could do - for hypot (x, y) see if we have
-x and use that, though it requires we simplify -tan(-x) to tan(x).

The simplest way to avoid the regressions seen is to not build a SAVE_EXPR
when arg0 is a COMPLEX_EXPR as we're then for sure folding - OTOH the
GENERIC folding seems premature as complex lowering should deal with this
as well.  Restricting the transform to arg0 being a COMPLEX_EXPR would
solve the issue as well.  Disabling it completely shows we optimize
the pr30172-1.c test even at -O0 during complex lowering.

[Bug middle-end/116454] [12/13/14/15 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice since r0-77972-g9f5396713d9e18

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

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

commit r15-3087-gb07f8a301158e53717b8688cc8ea430b6f02574c
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Wed Aug 21 17:41:38 2024 -0700

    fold: Fix `a * 1j` if a has side effects [PR116454]

    The problem here was a missing save_expr around arg0 since
    it is used twice, once in REALPART_EXPR and once in IMAGPART_EXPR.
    Thia adds the save_expr and reformats the code slightly so it is a
    little easier to understand.  It excludes the case when arg0 is
    a COMPLEX_EXPR since in that case we'll end up with the distinct
    real and imaginary parts.  This is important to retain early
    optimization in some testcases.

    Bootstapped and tested on x86_64-linux-gnu with no regressions.

            PR middle-end/116454

    gcc/ChangeLog:

            * fold-const.cc (fold_binary_loc): Fix `a * +-1i`
            by wrapping arg0 with save_expr when it is not COMPLEX_EXPR.

    gcc/testsuite/ChangeLog:

            * gcc.dg/torture/pr116454-1.c: New test.
            * gcc.dg/torture/pr116454-2.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
    Co-Authored-By: Richard Biener  <rguenther@suse.de>

[Bug middle-end/116454] [12/13/14 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice since r0-77972-g9f5396713d9e18

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |15.0
            Summary|[12/13/14/15 Regression]    |[12/13/14 Regression]
                   |`*++ptr * 1j` is handled    |`*++ptr * 1j` is handled
                   |incorrectly with            |incorrectly with
                   |-ffast-math, increments ptr |-ffast-math, increments ptr
                   |twice since                 |twice since
                   |r0-77972-g9f5396713d9e18    |r0-77972-g9f5396713d9e18

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug middle-end/116454] [12/13/14 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice since r0-77972-g9f5396713d9e18

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-14 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:12c00048d9f3598e57b98ec7723f7356bd255d04

commit r14-10683-g12c00048d9f3598e57b98ec7723f7356bd255d04
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Wed Aug 21 17:41:38 2024 -0700

    fold: Fix `a * 1j` if a has side effects [PR116454]

    The problem here was a missing save_expr around arg0 since
    it is used twice, once in REALPART_EXPR and once in IMAGPART_EXPR.
    Thia adds the save_expr and reformats the code slightly so it is a
    little easier to understand.  It excludes the case when arg0 is
    a COMPLEX_EXPR since in that case we'll end up with the distinct
    real and imaginary parts.  This is important to retain early
    optimization in some testcases.

    Bootstapped and tested on x86_64-linux-gnu with no regressions.

            PR middle-end/116454

    gcc/ChangeLog:

            * fold-const.cc (fold_binary_loc): Fix `a * +-1i`
            by wrapping arg0 with save_expr when it is not COMPLEX_EXPR.

    gcc/testsuite/ChangeLog:

            * gcc.dg/torture/pr116454-1.c: New test.
            * gcc.dg/torture/pr116454-2.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
    Co-Authored-By: Richard Biener  <rguenther@suse.de>
    (cherry picked from commit b07f8a301158e53717b8688cc8ea430b6f02574c)

[Bug middle-end/116454] [12/13 Regression] `*++ptr * 1j` is handled incorrectly with -ffast-math, increments ptr twice since r0-77972-g9f5396713d9e18

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |14.2.1
            Summary|[12/13/14 Regression]       |[12/13 Regression] `*++ptr
                   |`*++ptr * 1j` is handled    |* 1j` is handled
                   |incorrectly with            |incorrectly with
                   |-ffast-math, increments ptr |-ffast-math, increments ptr
                   |twice since                 |twice since
                   |r0-77972-g9f5396713d9e18    |r0-77972-g9f5396713d9e18
      Known to fail|                            |14.2.0
           Priority|P3                          |P2