public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/90248] [8/9/10/11 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
@ 2021-01-21 18:46 ` jakub at gcc dot gnu.org
  2021-01-22 10:51 ` cvs-commit at gcc dot gnu.org
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-21 18:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50024
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50024&action=edit
gcc11-pr90248.patch

So, I think at minimum we want this, i.e. turn the bogus simplifications into
simplifications directly into +-abs if the conditions are multiplied by X.
That should work fine even for +/-0.0, except that it perhaps could get the
sign of zero wrong (which is why I've kept it guarded on !HONOR_SIGNED_ZEROS).
Additionally, we can consider (phiopt?) optimization that will optimize the VCE
to signed int ? const : -const into copysign.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9/10/11 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
  2021-01-21 18:46 ` [Bug middle-end/90248] [8/9/10/11 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations jakub at gcc dot gnu.org
@ 2021-01-22 10:51 ` cvs-commit at gcc dot gnu.org
  2021-01-22 10:52 ` [Bug middle-end/90248] [8/9/10 " jakub at gcc dot gnu.org
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-01-22 10:51 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from CVS 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:36fe1cdc9534c36c02803ce97557130354d2b2a0

commit r11-6853-g36fe1cdc9534c36c02803ce97557130354d2b2a0
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jan 22 11:50:18 2021 +0100

    match.pd: Replace incorrect simplifications into copysign [PR90248]

    In the PR Andrew said he has implemented a simplification that has been
    added to LLVM, but that actually is not true, what is in there are
    X * (X cmp 0.0 ? +-1.0 : -+1.0) simplifications into +-abs(X)
    but what has been added into GCC are (X cmp 0.0 ? +-1.0 : -+1.0)
    simplifications into copysign(1, +-X) and then
    X * copysign (1, +-X) into +-abs (X).
    The problem is with the (X cmp 0.0 ? +-1.0 : -+1.0) simplifications,
    they don't work correctly when X is zero.
    E.g.
    (X > 0.0 ? 1.0 : -1.0)
    is -1.0 when X is either -0.0 or 0.0, but copysign will make it return
    1.0 for 0.0 and -1.0 only for -0.0.
    (X >= 0.0 ? 1.0 : -1.0)
    is 1.0 when X is either -0.0 or 0.0, but copysign will make it return
    still 1.0 for 0.0 and -1.0 for -0.0.
    The simplifications were guarded on !HONOR_SIGNED_ZEROS, but as discussed
in
    the PR, that option doesn't mean that -0.0 will not ever appear as operand
    of some operation, it is hard to guarantee that without compiler adding
    canonicalizations of -0.0 to 0.0 after most of the operations and thus
    making it very slow, but that the user asserts that he doesn't care if the
result
    of operations will be 0.0 or -0.0.  Not to mention that some of the
    transformations are incorrect even for positive 0.0.

    So, instead of those simplifications this patch recognizes patterns where
    those ?: expressions are multiplied by X, directly into +-abs.
    That works fine even for 0.0 and -0.0 (as long as we don't care about
    whether the result is exactly 0.0 or -0.0 in those cases), because
    whether the result of copysign is -1.0 or 1.0 doesn't matter when it is
    multiplied by 0.0 or -0.0.

    As a follow-up, maybe we should add the simplification mentioned in the PR,
    in particular doing copysign by hand through
    VIEW_CONVERT_EXPR <int, float_X> < 0 ? -float_constant : float_constant
    into copysign (float_constant, float_X).  But I think that would need to be
    done in phiopt.

    2021-01-22  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/90248
            * match.pd (X cmp 0.0 ? 1.0 : -1.0 -> copysign(1, +-X),
            X cmp 0.0 ? -1.0 : +1.0 -> copysign(1, -+X)): Remove
            simplifications.
            (X * (X cmp 0.0 ? 1.0 : -1.0) -> +-abs(X),
            X * (X cmp 0.0 ? -1.0 : 1.0) -> +-abs(X)): New simplifications.

            * gcc.dg/tree-ssa/copy-sign-1.c: Don't expect any copysign
            builtins.
            * gcc.dg/pr90248.c: New test.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9/10 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
  2021-01-21 18:46 ` [Bug middle-end/90248] [8/9/10/11 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations jakub at gcc dot gnu.org
  2021-01-22 10:51 ` cvs-commit at gcc dot gnu.org
@ 2021-01-22 10:52 ` jakub at gcc dot gnu.org
  2021-01-29 19:19 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-22 10:52 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10/11 Regression]      |[8/9/10 Regression] larger
                   |larger than 0 compare fails |than 0 compare fails with
                   |with -ffinite-math-only     |-ffinite-math-only
                   |-funsafe-math-optimizations |-funsafe-math-optimizations

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9/10 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
                   ` (2 preceding siblings ...)
  2021-01-22 10:52 ` [Bug middle-end/90248] [8/9/10 " jakub at gcc dot gnu.org
@ 2021-01-29 19:19 ` cvs-commit at gcc dot gnu.org
  2021-01-29 19:23 ` [Bug middle-end/90248] [8/9 " jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-01-29 19:19 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

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

commit r10-9317-gdd92986ea6d2d363146e1726817a84910453fdc8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jan 22 11:50:18 2021 +0100

    match.pd: Replace incorrect simplifications into copysign [PR90248]

    In the PR Andrew said he has implemented a simplification that has been
    added to LLVM, but that actually is not true, what is in there are
    X * (X cmp 0.0 ? +-1.0 : -+1.0) simplifications into +-abs(X)
    but what has been added into GCC are (X cmp 0.0 ? +-1.0 : -+1.0)
    simplifications into copysign(1, +-X) and then
    X * copysign (1, +-X) into +-abs (X).
    The problem is with the (X cmp 0.0 ? +-1.0 : -+1.0) simplifications,
    they don't work correctly when X is zero.
    E.g.
    (X > 0.0 ? 1.0 : -1.0)
    is -1.0 when X is either -0.0 or 0.0, but copysign will make it return
    1.0 for 0.0 and -1.0 only for -0.0.
    (X >= 0.0 ? 1.0 : -1.0)
    is 1.0 when X is either -0.0 or 0.0, but copysign will make it return
    still 1.0 for 0.0 and -1.0 for -0.0.
    The simplifications were guarded on !HONOR_SIGNED_ZEROS, but as discussed
in
    the PR, that option doesn't mean that -0.0 will not ever appear as operand
    of some operation, it is hard to guarantee that without compiler adding
    canonicalizations of -0.0 to 0.0 after most of the operations and thus
    making it very slow, but that the user asserts that he doesn't care if the
result
    of operations will be 0.0 or -0.0.  Not to mention that some of the
    transformations are incorrect even for positive 0.0.

    So, instead of those simplifications this patch recognizes patterns where
    those ?: expressions are multiplied by X, directly into +-abs.
    That works fine even for 0.0 and -0.0 (as long as we don't care about
    whether the result is exactly 0.0 or -0.0 in those cases), because
    whether the result of copysign is -1.0 or 1.0 doesn't matter when it is
    multiplied by 0.0 or -0.0.

    As a follow-up, maybe we should add the simplification mentioned in the PR,
    in particular doing copysign by hand through
    VIEW_CONVERT_EXPR <int, float_X> < 0 ? -float_constant : float_constant
    into copysign (float_constant, float_X).  But I think that would need to be
    done in phiopt.

    2021-01-22  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/90248
            * match.pd (X cmp 0.0 ? 1.0 : -1.0 -> copysign(1, +-X),
            X cmp 0.0 ? -1.0 : +1.0 -> copysign(1, -+X)): Remove
            simplifications.
            (X * (X cmp 0.0 ? 1.0 : -1.0) -> +-abs(X),
            X * (X cmp 0.0 ? -1.0 : 1.0) -> +-abs(X)): New simplifications.

            * gcc.dg/tree-ssa/copy-sign-1.c: Don't expect any copysign
            builtins.
            * gcc.dg/pr90248.c: New test.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
                   ` (3 preceding siblings ...)
  2021-01-29 19:19 ` cvs-commit at gcc dot gnu.org
@ 2021-01-29 19:23 ` jakub at gcc dot gnu.org
  2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-29 19:23 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10 Regression] larger  |[8/9 Regression] larger
                   |than 0 compare fails with   |than 0 compare fails with
                   |-ffinite-math-only          |-ffinite-math-only
                   |-funsafe-math-optimizations |-funsafe-math-optimizations

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.3+ too.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
                   ` (4 preceding siblings ...)
  2021-01-29 19:23 ` [Bug middle-end/90248] [8/9 " jakub at gcc dot gnu.org
@ 2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
  2021-04-22 16:49 ` cvs-commit at gcc dot gnu.org
  2021-04-22 17:05 ` jakub at gcc dot gnu.org
  7 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-20 23:31 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:45a6eae129c6fee387a3bb7075181d8509fa6e2a

commit r9-9407-g45a6eae129c6fee387a3bb7075181d8509fa6e2a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jan 22 11:50:18 2021 +0100

    match.pd: Replace incorrect simplifications into copysign [PR90248]

    In the PR Andrew said he has implemented a simplification that has been
    added to LLVM, but that actually is not true, what is in there are
    X * (X cmp 0.0 ? +-1.0 : -+1.0) simplifications into +-abs(X)
    but what has been added into GCC are (X cmp 0.0 ? +-1.0 : -+1.0)
    simplifications into copysign(1, +-X) and then
    X * copysign (1, +-X) into +-abs (X).
    The problem is with the (X cmp 0.0 ? +-1.0 : -+1.0) simplifications,
    they don't work correctly when X is zero.
    E.g.
    (X > 0.0 ? 1.0 : -1.0)
    is -1.0 when X is either -0.0 or 0.0, but copysign will make it return
    1.0 for 0.0 and -1.0 only for -0.0.
    (X >= 0.0 ? 1.0 : -1.0)
    is 1.0 when X is either -0.0 or 0.0, but copysign will make it return
    still 1.0 for 0.0 and -1.0 for -0.0.
    The simplifications were guarded on !HONOR_SIGNED_ZEROS, but as discussed
in
    the PR, that option doesn't mean that -0.0 will not ever appear as operand
    of some operation, it is hard to guarantee that without compiler adding
    canonicalizations of -0.0 to 0.0 after most of the operations and thus
    making it very slow, but that the user asserts that he doesn't care if the
result
    of operations will be 0.0 or -0.0.  Not to mention that some of the
    transformations are incorrect even for positive 0.0.

    So, instead of those simplifications this patch recognizes patterns where
    those ?: expressions are multiplied by X, directly into +-abs.
    That works fine even for 0.0 and -0.0 (as long as we don't care about
    whether the result is exactly 0.0 or -0.0 in those cases), because
    whether the result of copysign is -1.0 or 1.0 doesn't matter when it is
    multiplied by 0.0 or -0.0.

    As a follow-up, maybe we should add the simplification mentioned in the PR,
    in particular doing copysign by hand through
    VIEW_CONVERT_EXPR <int, float_X> < 0 ? -float_constant : float_constant
    into copysign (float_constant, float_X).  But I think that would need to be
    done in phiopt.

    2021-01-22  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/90248
            * match.pd (X cmp 0.0 ? 1.0 : -1.0 -> copysign(1, +-X),
            X cmp 0.0 ? -1.0 : +1.0 -> copysign(1, -+X)): Remove
            simplifications.
            (X * (X cmp 0.0 ? 1.0 : -1.0) -> +-abs(X),
            X * (X cmp 0.0 ? -1.0 : 1.0) -> +-abs(X)): New simplifications.

            * gcc.dg/tree-ssa/copy-sign-1.c: Don't expect any copysign
            builtins.
            * gcc.dg/pr90248.c: New test.

    (cherry picked from commit dd92986ea6d2d363146e1726817a84910453fdc8)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
                   ` (5 preceding siblings ...)
  2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
@ 2021-04-22 16:49 ` cvs-commit at gcc dot gnu.org
  2021-04-22 17:05 ` jakub at gcc dot gnu.org
  7 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-22 16:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:4ce16d154dd3d34e72117971564b97a388935a12

commit r8-10874-g4ce16d154dd3d34e72117971564b97a388935a12
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jan 22 11:50:18 2021 +0100

    match.pd: Replace incorrect simplifications into copysign [PR90248]

    In the PR Andrew said he has implemented a simplification that has been
    added to LLVM, but that actually is not true, what is in there are
    X * (X cmp 0.0 ? +-1.0 : -+1.0) simplifications into +-abs(X)
    but what has been added into GCC are (X cmp 0.0 ? +-1.0 : -+1.0)
    simplifications into copysign(1, +-X) and then
    X * copysign (1, +-X) into +-abs (X).
    The problem is with the (X cmp 0.0 ? +-1.0 : -+1.0) simplifications,
    they don't work correctly when X is zero.
    E.g.
    (X > 0.0 ? 1.0 : -1.0)
    is -1.0 when X is either -0.0 or 0.0, but copysign will make it return
    1.0 for 0.0 and -1.0 only for -0.0.
    (X >= 0.0 ? 1.0 : -1.0)
    is 1.0 when X is either -0.0 or 0.0, but copysign will make it return
    still 1.0 for 0.0 and -1.0 for -0.0.
    The simplifications were guarded on !HONOR_SIGNED_ZEROS, but as discussed
in
    the PR, that option doesn't mean that -0.0 will not ever appear as operand
    of some operation, it is hard to guarantee that without compiler adding
    canonicalizations of -0.0 to 0.0 after most of the operations and thus
    making it very slow, but that the user asserts that he doesn't care if the
result
    of operations will be 0.0 or -0.0.  Not to mention that some of the
    transformations are incorrect even for positive 0.0.

    So, instead of those simplifications this patch recognizes patterns where
    those ?: expressions are multiplied by X, directly into +-abs.
    That works fine even for 0.0 and -0.0 (as long as we don't care about
    whether the result is exactly 0.0 or -0.0 in those cases), because
    whether the result of copysign is -1.0 or 1.0 doesn't matter when it is
    multiplied by 0.0 or -0.0.

    As a follow-up, maybe we should add the simplification mentioned in the PR,
    in particular doing copysign by hand through
    VIEW_CONVERT_EXPR <int, float_X> < 0 ? -float_constant : float_constant
    into copysign (float_constant, float_X).  But I think that would need to be
    done in phiopt.

    2021-01-22  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/90248
            * match.pd (X cmp 0.0 ? 1.0 : -1.0 -> copysign(1, +-X),
            X cmp 0.0 ? -1.0 : +1.0 -> copysign(1, -+X)): Remove
            simplifications.
            (X * (X cmp 0.0 ? 1.0 : -1.0) -> +-abs(X),
            X * (X cmp 0.0 ? -1.0 : 1.0) -> +-abs(X)): New simplifications.

            * gcc.dg/tree-ssa/copy-sign-1.c: Don't expect any copysign
            builtins.
            * gcc.dg/pr90248.c: New test.

    (cherry picked from commit dd92986ea6d2d363146e1726817a84910453fdc8)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Bug middle-end/90248] [8/9 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations
       [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
                   ` (6 preceding siblings ...)
  2021-04-22 16:49 ` cvs-commit at gcc dot gnu.org
@ 2021-04-22 17:05 ` jakub at gcc dot gnu.org
  7 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-22 17:05 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
           Assignee|pinskia at gcc dot gnu.org         |jakub at gcc dot gnu.org
             Status|ASSIGNED                    |RESOLVED

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-04-22 17:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <bug-90248-4@http.gcc.gnu.org/bugzilla/>
2021-01-21 18:46 ` [Bug middle-end/90248] [8/9/10/11 Regression] larger than 0 compare fails with -ffinite-math-only -funsafe-math-optimizations jakub at gcc dot gnu.org
2021-01-22 10:51 ` cvs-commit at gcc dot gnu.org
2021-01-22 10:52 ` [Bug middle-end/90248] [8/9/10 " jakub at gcc dot gnu.org
2021-01-29 19:19 ` cvs-commit at gcc dot gnu.org
2021-01-29 19:23 ` [Bug middle-end/90248] [8/9 " jakub at gcc dot gnu.org
2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
2021-04-22 16:49 ` cvs-commit at gcc dot gnu.org
2021-04-22 17:05 ` jakub at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).