[Bug target/96854] New: avx vectorizer breaks complex arithmetic

[Bug target/96854] New: avx vectorizer breaks complex arithmetic

[already5chosen at yahoo dot com]

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

            Bug ID: 96854
           Summary: avx vectorizer breaks complex arithmetic
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: already5chosen at yahoo dot com
  Target Milestone: ---

'-Ofast -mavx -march=ivybridge' miscompiles this simple loop:

double complex foo(double complex acc, const double complex *x, const double
complex* y, int N)
{
  for (int c = 0; c < N; ++c)
    acc -= x[c] * y[c];
  return acc;
}

The bug appears to be triggered by -fassociative-math, but it could be a
trigger rather than the reason.

You can find a reproducer here:
https://github.com/already5chosen/others/tree/master/cholesky_solver/gcc-bug
I only tried with MSYS2 variant of gcc 10.2.0

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |needs-bisection
      Known to fail|                            |10.1.0, 10.2.0
      Known to work|                            |11.0, 9.3.0
     Ever confirmed|0                           |1
   Target Milestone|---                         |10.3
   Last reconfirmed|                            |2020-08-31
             Target|                            |x86_64-*-* i?86-*-*
            Summary|avx vectorizer breaks       |[10 Regression] avx
                   |complex arithmetic          |vectorizer breaks complex
                   |                            |arithmetic
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
#include <complex.h>

double complex __attribute__((noipa))
foo(double complex acc, const double complex *x, const double complex* y, int
N)
{
  for (int c = 0; c < N; ++c)
    acc -= x[c] * y[c];
  return acc;
}

int main(void)
{
  static const double complex y[] = { 1, 2, };
  static const double complex x[] = { 1, 3, };

  double complex ref = foo(0, x, y, 2); // reference

  if (creal(ref) != -7.)
    __builtin_abort ();
  return 0;
}


Confirmed with GCC 10.2, it works on trunk and with GCC 9.3.0.

It doesn't need any -march for me, just -Ofast -mavx is enough.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection             |
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
double _Complex __attribute__((noipa))
foo (double _Complex acc, const double _Complex *x, const double _Complex* y,
int N)
{
  for (int c = 0; c < N; ++c)
    acc -= x[c] * y[c];
  return acc;
}

int
main()
{
  static const double _Complex y[] = { 1, 2, };
  static const double _Complex x[] = { 1, 3, };
  double _Complex ref = foo (0, x, y, 2);
  if (__builtin_creal (ref) != -7.)
    __builtin_abort ();
  return 0;
}

Started with r10-3252-g901083b9bdf69a7b1382f9682c6fd1d5759667dd
stopped with r11-1501-gda2b7c7f0a136b4d00520a08d4c443fc2e3a467d
The result it compiles when miscompiled is 7., so it is not just some ulp
difference in the computation.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenth at gcc dot gnu.org]

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

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
           Priority|P3                          |P2

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

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[already5chosen at yahoo dot com]

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

--- Comment #4 from Michael_S <already5chosen at yahoo dot com> ---
Pay attention that it's not just AVX.
'-mavx2 -mfma -Ofast' generates different code, but at the end gives the same
wrong result.
Unfortunately, I have no AVX512 hardware to test, but wouldn't be surprised if
it's also broken in 10.2.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK so the issue is that SLP_TREE_TWO_OPERATORS as it used to be cannot be used
to drive live operation vectorization (it does it twice but with only the
intermediate vector results).  The easiest is to not try to fix this (the issue
is latent everywhere) and simply punt on such live operations.  That will
force non-SLP vectorization.

The triggering rev. opened up the SLP vectorization opportunity and wasn't
at fault itself.

Testing patch.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Michael_S from comment #4)
> Pay attention that it's not just AVX.
> '-mavx2 -mfma -Ofast' generates different code, but at the end gives the
> same wrong result.
> Unfortunately, I have no AVX512 hardware to test, but wouldn't be surprised
> if it's also broken in 10.2.

Even SSE codegen is broken, you just need -fno-vect-cost-model to trigger it
there.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
On trunk we fail the SLP reduction vectorization because a VEC_PERM SLP
reduction
operation is not supported by epilogue generation.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:9f980cdba9e2fc0cc3f50c2c790f53b4dcd9dbe5

commit r10-8693-g9f980cdba9e2fc0cc3f50c2c790f53b4dcd9dbe5
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 31 13:36:09 2020 +0200

    tree-optimization/96854 - SLP reduction of two-operator is broken

    This fixes SLP reduction of two-operator operations by marking those
    not supported.  In fact any live lane out of such an operation cannot
    be code-generated correctly.

    2020-08-31  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/96854
            * tree-vect-loop.c (vectorizable_live_operation): Disallow
            SLP_TREE_TWO_OPERATORS nodes.

            * gcc.dg/vect/pr96854.c: New testcase.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS 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:f089569851ca9c8a81400dd8a159f86636ed20ec

commit r11-2949-gf089569851ca9c8a81400dd8a159f86636ed20ec
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 31 13:36:09 2020 +0200

    tree-optimization/96854 - testcase for SLP reduction of two-operator

    This adds the testcase for the already fixed PR.

    2020-08-31  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/96854
            * gcc.dg/vect/pr96854.c: New testcase.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[already5chosen at yahoo dot com]

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

--- Comment #11 from Michael_S <already5chosen at yahoo dot com> ---
Just to understand
Will 10.1 and 10.2 be fixed?

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Michael_S from comment #11)
> Just to understand
> Will 10.1 and 10.2 be fixed?

No, they were already released, so can't be fixed.  You can apply the changes
to them if you want.  The fix is on the git releases/gcc-10 branch, and will be
part of upcoming 10.3.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[already5chosen at yahoo dot com]

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

--- Comment #13 from Michael_S <already5chosen at yahoo dot com> ---
I don't follow gcc versioning policy all that closely.
What is the function "micro" versions now? For internal use and experimentation
only, but not for public release?

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[jakub at gcc dot gnu.org]

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The versioning is well documented https://gcc.gnu.org/develop.html#num_scheme
and the only way how things are fixed is fixing on the mainline and when needed
on still open release branches.  Those will become an upcoming release at some
later point.  Generally, it is advisable to use snapshots from the release
branches, as otherwise one misses dozens to hundreds of bugfixes that were
fixed since the last release.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[already5chosen at yahoo dot com]

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

--- Comment #15 from Michael_S <already5chosen at yahoo dot com> ---
Thank you.
That does not sound too different from what I assumed in post above.
10.1.0 is release. Expected to be used by "normal" people.
10.1.1 was for purpose of development of 10.2.0. Since release of 10.2.0 it is
obsolete.
10.2.0 is release. Expected to be used by "normal" people.
10.2.1 exists for purpose of development of 10.3.0.

>> Generally, it is advisable to use snapshots from the release branches, as otherwise one misses dozens to hundreds of bugfixes that were fixed since the last release.

That a little confusing.
I am compiler user, not compiler developer. Is it advisable for me to download
and compile snapshots ?!
I would think that for people like me 3-4 month cadence of gcc releases is
already too quick.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[jakub at gcc dot gnu.org]

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If you want bugfixes as in this case.  The cadence on the release branches is
gradually slowing down, the last release is usually about a year about the one
before that.  It is already quite a lot of work to backport fixes to release
branches, maintaining some 10.1 and 10.2 etc. streams would be significantly
more, plus usually you want all bugfixes from the release branch, i.e. there
would be no difference between 10.2.fixes and 10.3.

[Bug target/96854] [10 Regression] avx vectorizer breaks complex arithmetic

[rguenther at suse dot de]

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

--- Comment #17 from rguenther at suse dot de <rguenther at suse dot de> ---
On Sun, 6 Sep 2020, already5chosen at yahoo dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96854
> 
> --- Comment #15 from Michael_S <already5chosen at yahoo dot com> ---
> Thank you.
> That does not sound too different from what I assumed in post above.
> 10.1.0 is release. Expected to be used by "normal" people.
> 10.1.1 was for purpose of development of 10.2.0. Since release of 10.2.0 it is
> obsolete.
> 10.2.0 is release. Expected to be used by "normal" people.
> 10.2.1 exists for purpose of development of 10.3.0.
> 
> >> Generally, it is advisable to use snapshots from the release branches, as otherwise one misses dozens to hundreds of bugfixes that were fixed since the last release.
> 
> That a little confusing.
> I am compiler user, not compiler developer. Is it advisable for me to download
> and compile snapshots ?!

If you can wait then no, 10.x.0 are the "releases".  If you are using
pre-built binaries from some Linux distribution or other distributor
then the distributor usually picks up the current branch head for you.

> I would think that for people like me 3-4 month cadence of gcc releases is
> already too quick.

Well, all of GCC 10.x.[01] are one major release with a major release
cadence of about one year.  Everything else inbetween is just accumulated
bugfixes with the "conveniece" of us doing 10.2.0 and later 10.3.0 at
random points to officially release a tarball with accumulated bugfixes.

You could say we should make it GCC 10.bugfix-nr and thus only have
two numbers ;)