* [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