[Bug fortran/115710] New: fortran complex abs does not vectorise

[Bug fortran/115710] New: fortran complex abs does not vectorise

[mjr19 at cam dot ac.uk]

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

            Bug ID: 115710
           Summary: fortran complex abs does not vectorise
           Product: gcc
           Version: 14.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mjr19 at cam dot ac.uk
  Target Milestone: ---

subroutine foo(a,b,n)
  complex(kind(1d0))::a(*)
  real(kind(1d0))::b(*)
  integer::i,n

  do i=1,n
     b(i)=abs(a(i))**2
  end do

end subroutine foo

fails to vectorise with "gfortran-14 -mavx2 -mfma -Ofast".

I am very impressed that the **2 simply removes the sqrt, so the above code is
simpler than the code without **2 (which also doesn't vectorise, and should). I
am also impressed that

  b(i)=a(i)*conjg(a(i))

and

  b(i)=real(a(i))*real(a(i))+aimag(a(i))*aimag(a(i))

produce identical, vectorised, assembler. But the abs example ought to be the
same too.

[Bug fortran/115710] fortran complex abs does not vectorise

[kargls at comcast dot net]

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

kargls at comcast dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kargls at comcast dot net

--- Comment #1 from kargls at comcast dot net ---
Looks like a wrong component was chosen.  I get nearly identical assembly with

#include <complex.h>
#include <math.h>

void
foo(double complex *a, double *b, int n)
{
  for (int i=0; i < n; i++)
     b[i] = cabs(a[i]) * cabs(a[i]);
}

~/work/bin/gcc      -S -mavx2 -mfma -Ofast -o zc.s a.c
~/work/bin/gfortran -S -mavx2 -mfma -Ofast -o zf.s -fno-underscore a.f90

[Bug tree-optimization/115710] fortran complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-06-29
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
/app/example.cpp:7:19: note:   === vect_analyze_data_refs ===
/app/example.cpp:7:19: missed:   not vectorized: no vectype for stmt: _4 = *_3;
 scalar_type: complex doubleD.46
/app/example.cpp:8:13: missed:   not vectorized: no vectype for stmt: _4 = *_3;
 scalar_type: complex double


Confirmed.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |5.5.0
            Summary|fortran complex abs does    |[11/12/13/14/15 Regression]
                   |not vectorise               |complex abs does not
                   |                            |vectorise
   Target Milestone|---                         |11.5
           Keywords|                            |needs-bisection
      Known to fail|                            |6.1.0

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So before forwprop3 we have:
```
  _10 = REALPART_EXPR <*_3>;
  _9 = IMAGPART_EXPR <*_3>;
  _4 = COMPLEX_EXPR <_10, _9>;
  _5 = cabs (_4);
  _6 = _1 * 8;
  _7 = b_14(D) + _6;
  reassocpow_18 = __builtin_powi (_5, 2);

```

After we get:
```
  _10 = REALPART_EXPR <*_3>;
  _4 = *_3;
  _9 = IMAGPART_EXPR <*_3>;
  _5 = cabs (_4);
  _6 = _1 * 8;
  _7 = b_14(D) + _6;
  reassocpow_18 = __builtin_powi (_5, 2);
```
Which undoes complex lowering.

But then powcabs does:
```
  _10 = REALPART_EXPR <*_3>;
  _4 = *_3;
  _9 = IMAGPART_EXPR <*_3>;
  cabs_8 = REALPART_EXPR <_4>;
  cabs_17 = cabs_8 * cabs_8;
  cabs_22 = IMAGPART_EXPR <_4>;
  cabs_23 = cabs_22 * cabs_22;
  cabs_24 = cabs_17 + cabs_23;
  powroot_25 = __builtin_sqrt (cabs_24);
  _5 = powroot_25;
  _6 = _1 * 8;
  _7 = b_14(D) + _6;
  powmult_26 = _5 * _5;
  reassocpow_18 = powmult_26;
  *_7 = reassocpow_18;
```
Which redoes some of the complex lowering but misses undoing the undo of
COMPLEX_EXPR<REAL<*3>, IMAG<*3>> ...

This is a regression even as GCC 5 used to be able top vectorize it.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think this is a pass ordering issue between complex lowering, forwprop and
powcabs.

This is also a missed CSE opportunity in PREs VN instance:

  _5 = REALPART_EXPR <(*a_9(D))[_1]>;
  _2 = (*a_9(D))[_1];
  _4 = IMAGPART_EXPR <(*a_9(D))[_1]>;
  cabs_11 = REALPART_EXPR <_2>;

here the realpart of _2 should have been CSEd to _5.

I wonder if we shouldn't do CABS expansion during complex lowering instead?
Both are technically to get closer to the target.

I'm putting it on my TODO but I don't expect to have cycles to look at
any of the above.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> I wonder if we shouldn't do CABS expansion during complex lowering instead?

It would be one less pass around the IR too so it could speed up the compiler
slightly. My todo list is too long right now too. Maybe I can put it above loop
flattening.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

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
           Keywords|needs-bisection             |

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> (In reply to Richard Biener from comment #4)
> > I wonder if we shouldn't do CABS expansion during complex lowering instead?
> 
> It would be one less pass around the IR too so it could speed up the
> compiler slightly. My todo list is too long right now too. Maybe I can put
> it above loop flattening.

Oh powcabs handles also powi expansion too so it won't remove the pass around
the IR.

Anyways I am going to take a stab at moving cabs expansion to complex lowering
where it should be instead.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

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

I will add a few testcases too.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I wonder if we could also add:
+/* cabs(x+0i) or cabs(0+xi) -> abs(x).  */
+(simplify
+ (CABS (complex:c @0 real_zerop@1))
+ (abs @0))


+ /* cabs(x+xi) -> fabs(x)*sqrt(2).  */
+ (simplify
+  (CABS (complex @0 @0))
+  (mult (abs @0) { build_real_truncate (type, dconst_sqrt2 ()); })))


to the lowering part.

let me look into doing that but it will be 3rd part to the patch series. Since
we should have this info already ...

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #8)
> I wonder if we could also add:
> +/* cabs(x+0i) or cabs(0+xi) -> abs(x).  */
> +(simplify
> + (CABS (complex:c @0 real_zerop@1))
> + (abs @0))
> 
> 
> + /* cabs(x+xi) -> fabs(x)*sqrt(2).  */
> + (simplify
> +  (CABS (complex @0 @0))
> +  (mult (abs @0) { build_real_truncate (type, dconst_sqrt2 ()); })))
> 
> 
> to the lowering part.
> 
> let me look into doing that but it will be 3rd part to the patch series.
> Since we should have this info already ...

This is needed to get the same code generation as before for:
```
double f(double a, double c)
{
  _Complex double b = a;
  b+= c;
  return __builtin_cabs(b);
}
```

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #9)
> (In reply to Andrew Pinski from comment #8)
> > I wonder if we could also add:
> > +/* cabs(x+0i) or cabs(0+xi) -> abs(x).  */
> > +(simplify
> > + (CABS (complex:c @0 real_zerop@1))
> > + (abs @0))
> > 
> > 
> > + /* cabs(x+xi) -> fabs(x)*sqrt(2).  */
> > + (simplify
> > +  (CABS (complex @0 @0))
> > +  (mult (abs @0) { build_real_truncate (type, dconst_sqrt2 ()); })))
> > 
> > 
> > to the lowering part.
> > 
> > let me look into doing that but it will be 3rd part to the patch series.
> > Since we should have this info already ...
> 
> This is needed to get the same code generation as before for:
> ```
> double f(double a, double c)
> {
>   _Complex double b = a;
>   b+= c;
>   return __builtin_cabs(b);
> }
> ```

And the other testcase that matters here:
```
double f(double a, double c)
{
  _Complex double d = __builtin_complex (a, a);
  d+=__builtin_complex(1.0, 1.0);
  return __builtin_cabs(d);
}

```

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[rguenther at suse dot de]

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

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Sun, 30 Jun 2024, pinskia at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115710
> 
> --- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> I wonder if we could also add:
> +/* cabs(x+0i) or cabs(0+xi) -> abs(x).  */
> +(simplify
> + (CABS (complex:c @0 real_zerop@1))
> + (abs @0))
> 
> 
> + /* cabs(x+xi) -> fabs(x)*sqrt(2).  */
> + (simplify
> +  (CABS (complex @0 @0))
> +  (mult (abs @0) { build_real_truncate (type, dconst_sqrt2 ()); })))
> 
> 
> to the lowering part.
> 
> let me look into doing that but it will be 3rd part to the patch series. Since
> we should have this info already ...

Complex lowering should use its lattice to do the usual simplifications,
sure.  Having those simplification patterns would be OK as well
of course.

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://inbox.sourceware.or
                   |                            |g/gcc-patches/2024070121465
                   |                            |7.3163066-1-quic_apinski@qu
                   |                            |icinc.com/T/#m57efaefcfff71
                   |                            |8bd712822ae490bb5aeba69a578

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patch series posted:
https://inbox.sourceware.org/gcc-patches/20240701214657.3163066-1-quic_apinski@quicinc.com/T/#m57efaefcfff718bd712822ae490bb5aeba69a578

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

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

commit r15-1797-gd8fe4f05ef448e6a136398d38c0f2170d3c6bb0d
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Sun Jun 30 12:57:14 2024 -0700

    Move cabs expansion from powcabs to complex lowering [PR115710]

    Expanding cabs in powcab might be too late as forwprop might
    recombine the load from a memory with the complex expr. Moving
    instead to complex lowering allows us to use directly the real/imag
    component from the loads instead. This allows for vectorization too.

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

            PR tree-optimization/115710

    gcc/ChangeLog:

            * tree-complex.cc (init_dont_simulate_again): Handle CABS.
            (gimple_expand_builtin_cabs): New function, moved mostly
            from tree-ssa-math-opts.cc.
            (expand_complex_operations_1): Call gimple_expand_builtin_cabs.
            * tree-ssa-math-opts.cc (gimple_expand_builtin_cabs): Remove.
            (build_and_insert_binop): Remove.
            (pass_data_expand_powcabs): Update comment.
            (pass_expand_powcabs::execute): Don't handle CABS.

    gcc/testsuite/ChangeLog:

            * gcc.dg/tree-ssa/cabs-1.c: New test.
            * gcc.dg/tree-ssa/cabs-2.c: New test.
            * gfortran.dg/vect/pr115710.f90: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

[Bug tree-optimization/115710] [11/12/13/14/15 Regression] complex abs does not vectorise

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|11.5                        |15.0

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed on the trunk, I don't think this should be backported; it has too many
moving parts of it.