[Bug tree-optimization/109829] New: Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[Bug tree-optimization/109829] New: Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[antoshkka at gmail dot com]

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

            Bug ID: 109829
           Summary: Optimizing __builtin_signbit(x) ? -x : x or abs for FP
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: antoshkka at gmail dot com
  Target Milestone: ---

Consider the following 2 functions:

__float128 abs1(__float128 x) { return __builtin_fabsf128(x); }
__float128 abs2(__float128 x) { return __builtin_signbit(x) ? -x : x; }

They should provide the same results, however the codegen is different:

abs1(__float128):
        pand    xmm0, XMMWORD PTR .LC0[rip]
        ret
abs2(__float128):
        movmskps        eax, xmm0
        test    al, 8
        je      .L4
        pxor    xmm0, XMMWORD PTR .LC1[rip]
.L4:
        ret


Looks like match.pd miss the __builtin_signbit(x) ? -x : x ->
__builtin_fabs*(x) pattern.

Playground: https://godbolt.org/z/bsxeozGqv

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
   Last reconfirmed|                            |2023-05-12
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
From phiopt1:
phiopt match-simplify trying:
        _1 != 0 ? iftmp.0_5 : x_3(D)

Where _1 is defined as:
  _1 = signbitD.1026 (x_3(D));
and iftmp.0_5 is defined as:
  iftmp.0_5 = -x_3(D);

This should fix it then:
(simplify
 (cond (ne (SIGNBIT @0) zero_p@1) (neg @0) @0)
 (abs @0))

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

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 implement this since it is only 3 lines or so. Just need some testcases.

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Should add this one too:

(simplify
 (cond (ne (SIGNBIT @0) zero_p@1) @0 (neg @0))
 (neg (abs @0)))

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Don't you need also
(simplify
 (cond (eq (SIGNBIT @0) zero_p@1) (neg @0) @0)
 (neg (abs @0)))
(simplify
 (cond (eq (SIGNBIT @0) zero_p@1) @0 (neg @0))
 (abs @0))
?
Or does match.pd try to invert all the COND_EXPR conditions and swap the
operands?

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or canonicalize COND_EXPRs such that only ne and not eq appears in condition
(and similarly to other comparisons)?

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> Or does match.pd try to invert all the COND_EXPR conditions and swap the
> operands?

match does not try but phiopt does in gimple_simplify_phiopt .

Or even something like this would work better. 
(for neeq (ne eq)
 (simplify
  (cond (neeq (SIGNBIT @0) zero_p@1) (neg @0) @0)
   (if (neqeq == NE_EXPR)
    (abs @0)
    (neg (abs @0))
   )
  )
 )
)

I need to double check if == 1 will show up here though.

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #6)
> I need to double check if == 1 will show up here though.

The only thing we know about __builtin_signbit is that is 0 or non-zero. The
exact value is unspecified for the non-zero case. So we can only optimize the
!=/== 0 cases.

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

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

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                URL|                            |https://gcc.gnu.org/piperma
                   |                            |il/gcc-patches/2023-May/618
                   |                            |435.html

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patch submitted:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618435.html

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS 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:82502b5c3463bde98d4b7ffb9ecef9b123799ed1

commit r14-813-g82502b5c3463bde98d4b7ffb9ecef9b123799ed1
Author: Andrew Pinski <apinski@marvell.com>
Date:   Sat May 13 22:25:21 2023 +0000

    MATCH: Add pattern for `signbit(x) ? x : -x` into abs (and swapped)

    This adds a simple pattern to match.pd for `signbit(x) ? x : -x`
    into abs<x>. This can be done for all types even ones that honor
    signed zeros and NaNs because both signbit and - are considered
    only looking at/touching the sign bit of those types and does
    not trap either.

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

            PR tree-optimization/109829

    gcc/ChangeLog:

            * match.pd: Add pattern for `signbit(x) !=/== 0 ? x : -x`.

    gcc/testsuite/ChangeLog:

            * gcc.dg/tree-ssa/abs-3.c: New test.
            * gcc.dg/tree-ssa/abs-4.c: New test.

[Bug tree-optimization/109829] Optimizing __builtin_signbit(x) ? -x : x or abs for FP

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.