* [PATCH] A == 0 ? A : -A same as -A (when A is 0.0)
@ 2022-08-29 20:44 Aldy Hernandez
2022-08-29 20:44 ` Aldy Hernandez
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Aldy Hernandez @ 2022-08-29 20:44 UTC (permalink / raw)
To: GCC patches; +Cc: Aldy Hernandez
The upcoming work for frange triggers a regression in
gcc.dg/tree-ssa/phi-opt-24.c.
For -O2 -fno-signed-zeros, we fail to transform the following into -A:
float f0(float A)
{
// A == 0? A : -A same as -A
if (A == 0) return A;
return -A;
}
This is because the abs/negative match.pd pattern here:
/* abs/negative simplifications moved from fold_cond_expr_with_comparison,
Need to handle (A - B) case as fold_cond_expr_with_comparison does.
Need to handle UN* comparisons.
...
...
Sees IL that has the 0.0 propagated.
Instead of:
<bb 2> [local count: 1073741824]:
if (A_2(D) == 0.0)
goto <bb 4>; [34.00%]
else
goto <bb 3>; [66.00%]
<bb 3> [local count: 708669601]:
_3 = -A_2(D);
<bb 4> [local count: 1073741824]:
# _1 = PHI <A_2(D)(2), _3(3)>
It now sees:
<bb 4> [local count: 1073741824]:
# _1 = PHI <0.0(2), _3(3)>
which it leaves untouched, causing the if conditional to survive.
Adding a variant to the pattern that for real_zerop fixes the problem.
I am a match.pd weenie, and am avoiding touching more patterns that
may also benefit from handling real constants. So please use simple
words if what I'm doing isn't correct ;-).
I did not include a testcase, as it's just phi-opt-24.c which will get
triggered when I commit the frange with endpoints work.
Tested on x86-64 & ppc64le Linux.
OK?
gcc/ChangeLog:
* match.pd ((cmp @0 zerop) real_zerop (negate@1 @0)): Add variant
for real zero.
---
gcc/match.pd | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gcc/match.pd b/gcc/match.pd
index 1bb936fc401..5bdea300f94 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4803,6 +4803,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(cnd (cmp @0 zerop) @0 (negate@1 @0))
(if (!HONOR_SIGNED_ZEROS (type))
@1))
+ (simplify
+ (cnd (cmp @0 zerop) real_zerop (negate@1 @0))
+ (if (!HONOR_SIGNED_ZEROS (type))
+ @1))
(simplify
(cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
(if (!HONOR_SIGNED_ZEROS (type))
--
2.37.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] A == 0 ? A : -A same as -A (when A is 0.0)
2022-08-29 20:44 [PATCH] A == 0 ? A : -A same as -A (when A is 0.0) Aldy Hernandez
@ 2022-08-29 20:44 ` Aldy Hernandez
2022-08-30 7:03 ` Richard Biener
2022-09-07 16:54 ` Jeff Law
2 siblings, 0 replies; 6+ messages in thread
From: Aldy Hernandez @ 2022-08-29 20:44 UTC (permalink / raw)
To: GCC patches
The upcoming work for frange triggers a regression in
gcc.dg/tree-ssa/phi-opt-24.c.
For -O2 -fno-signed-zeros, we fail to transform the following into -A:
float f0(float A)
{
// A == 0? A : -A same as -A
if (A == 0) return A;
return -A;
}
This is because the abs/negative match.pd pattern here:
/* abs/negative simplifications moved from fold_cond_expr_with_comparison,
Need to handle (A - B) case as fold_cond_expr_with_comparison does.
Need to handle UN* comparisons.
...
...
Sees IL that has the 0.0 propagated.
Instead of:
<bb 2> [local count: 1073741824]:
if (A_2(D) == 0.0)
goto <bb 4>; [34.00%]
else
goto <bb 3>; [66.00%]
<bb 3> [local count: 708669601]:
_3 = -A_2(D);
<bb 4> [local count: 1073741824]:
# _1 = PHI <A_2(D)(2), _3(3)>
It now sees:
<bb 4> [local count: 1073741824]:
# _1 = PHI <0.0(2), _3(3)>
which it leaves untouched, causing the if conditional to survive.
Adding a variant to the pattern that for real_zerop fixes the problem.
I am a match.pd weenie, and am avoiding touching more patterns that
may also benefit from handling real constants. So please use simple
words if what I'm doing isn't correct ;-).
I did not include a testcase, as it's just phi-opt-24.c which will get
triggered when I commit the frange with endpoints work.
Tested on x86-64 & ppc64le Linux.
OK?
gcc/ChangeLog:
* match.pd ((cmp @0 zerop) real_zerop (negate@1 @0)): Add variant
for real zero.
---
gcc/match.pd | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gcc/match.pd b/gcc/match.pd
index 1bb936fc401..5bdea300f94 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4803,6 +4803,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(cnd (cmp @0 zerop) @0 (negate@1 @0))
(if (!HONOR_SIGNED_ZEROS (type))
@1))
+ (simplify
+ (cnd (cmp @0 zerop) real_zerop (negate@1 @0))
+ (if (!HONOR_SIGNED_ZEROS (type))
+ @1))
(simplify
(cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
(if (!HONOR_SIGNED_ZEROS (type))
--
2.37.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] A == 0 ? A : -A same as -A (when A is 0.0)
2022-08-29 20:44 [PATCH] A == 0 ? A : -A same as -A (when A is 0.0) Aldy Hernandez
2022-08-29 20:44 ` Aldy Hernandez
@ 2022-08-30 7:03 ` Richard Biener
2022-08-30 7:06 ` Aldy Hernandez
2022-09-07 16:54 ` Jeff Law
2 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2022-08-30 7:03 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: GCC patches
On Mon, Aug 29, 2022 at 10:45 PM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The upcoming work for frange triggers a regression in
> gcc.dg/tree-ssa/phi-opt-24.c.
>
> For -O2 -fno-signed-zeros, we fail to transform the following into -A:
>
> float f0(float A)
> {
> // A == 0? A : -A same as -A
> if (A == 0) return A;
> return -A;
> }
>
> This is because the abs/negative match.pd pattern here:
>
> /* abs/negative simplifications moved from fold_cond_expr_with_comparison,
> Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> Need to handle UN* comparisons.
> ...
> ...
>
> Sees IL that has the 0.0 propagated.
>
> Instead of:
>
> <bb 2> [local count: 1073741824]:
> if (A_2(D) == 0.0)
> goto <bb 4>; [34.00%]
> else
> goto <bb 3>; [66.00%]
>
> <bb 3> [local count: 708669601]:
> _3 = -A_2(D);
>
> <bb 4> [local count: 1073741824]:
> # _1 = PHI <A_2(D)(2), _3(3)>
>
> It now sees:
>
> <bb 4> [local count: 1073741824]:
> # _1 = PHI <0.0(2), _3(3)>
>
> which it leaves untouched, causing the if conditional to survive.
>
> Adding a variant to the pattern that for real_zerop fixes the problem.
>
> I am a match.pd weenie, and am avoiding touching more patterns that
> may also benefit from handling real constants. So please use simple
> words if what I'm doing isn't correct ;-).
>
> I did not include a testcase, as it's just phi-opt-24.c which will get
> triggered when I commit the frange with endpoints work.
>
> Tested on x86-64 & ppc64le Linux.
>
> OK?
>
> gcc/ChangeLog:
>
> * match.pd ((cmp @0 zerop) real_zerop (negate@1 @0)): Add variant
> for real zero.
> ---
> gcc/match.pd | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 1bb936fc401..5bdea300f94 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4803,6 +4803,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (cnd (cmp @0 zerop) @0 (negate@1 @0))
> (if (!HONOR_SIGNED_ZEROS (type))
> @1))
> + (simplify
> + (cnd (cmp @0 zerop) real_zerop (negate@1 @0))
> + (if (!HONOR_SIGNED_ZEROS (type))
> + @1))
> (simplify
> (cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
> (if (!HONOR_SIGNED_ZEROS (type))
Looking at zerop:
bool
zerop (const_tree expr)
{
return (integer_zerop (expr)
|| real_zerop (expr)
|| fixed_zerop (expr));
}
your patch is equivalent to changing the integer_zerop one to use zerop?
(well, with the exception of also covering fixed_zerop).
OK if doing it that way.
Thanks,
Richard.
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] A == 0 ? A : -A same as -A (when A is 0.0)
2022-08-30 7:03 ` Richard Biener
@ 2022-08-30 7:06 ` Aldy Hernandez
2022-08-30 7:12 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Aldy Hernandez @ 2022-08-30 7:06 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC patches, MacLeod, Andrew
On Tue, Aug 30, 2022 at 9:03 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, Aug 29, 2022 at 10:45 PM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > The upcoming work for frange triggers a regression in
> > gcc.dg/tree-ssa/phi-opt-24.c.
> >
> > For -O2 -fno-signed-zeros, we fail to transform the following into -A:
> >
> > float f0(float A)
> > {
> > // A == 0? A : -A same as -A
> > if (A == 0) return A;
> > return -A;
> > }
> >
> > This is because the abs/negative match.pd pattern here:
> >
> > /* abs/negative simplifications moved from fold_cond_expr_with_comparison,
> > Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> > Need to handle UN* comparisons.
> > ...
> > ...
> >
> > Sees IL that has the 0.0 propagated.
> >
> > Instead of:
> >
> > <bb 2> [local count: 1073741824]:
> > if (A_2(D) == 0.0)
> > goto <bb 4>; [34.00%]
> > else
> > goto <bb 3>; [66.00%]
> >
> > <bb 3> [local count: 708669601]:
> > _3 = -A_2(D);
> >
> > <bb 4> [local count: 1073741824]:
> > # _1 = PHI <A_2(D)(2), _3(3)>
> >
> > It now sees:
> >
> > <bb 4> [local count: 1073741824]:
> > # _1 = PHI <0.0(2), _3(3)>
> >
> > which it leaves untouched, causing the if conditional to survive.
> >
> > Adding a variant to the pattern that for real_zerop fixes the problem.
> >
> > I am a match.pd weenie, and am avoiding touching more patterns that
> > may also benefit from handling real constants. So please use simple
> > words if what I'm doing isn't correct ;-).
> >
> > I did not include a testcase, as it's just phi-opt-24.c which will get
> > triggered when I commit the frange with endpoints work.
> >
> > Tested on x86-64 & ppc64le Linux.
> >
> > OK?
> >
> > gcc/ChangeLog:
> >
> > * match.pd ((cmp @0 zerop) real_zerop (negate@1 @0)): Add variant
> > for real zero.
> > ---
> > gcc/match.pd | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 1bb936fc401..5bdea300f94 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -4803,6 +4803,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > (cnd (cmp @0 zerop) @0 (negate@1 @0))
> > (if (!HONOR_SIGNED_ZEROS (type))
> > @1))
> > + (simplify
> > + (cnd (cmp @0 zerop) real_zerop (negate@1 @0))
> > + (if (!HONOR_SIGNED_ZEROS (type))
> > + @1))
> > (simplify
> > (cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
> > (if (!HONOR_SIGNED_ZEROS (type))
>
> Looking at zerop:
>
> bool
> zerop (const_tree expr)
> {
> return (integer_zerop (expr)
> || real_zerop (expr)
> || fixed_zerop (expr));
> }
>
> your patch is equivalent to changing the integer_zerop one to use zerop?
> (well, with the exception of also covering fixed_zerop).
Heh. That was my first instinct, but I thought fixed_zerop didn't
apply? No clue...I'm happy to change it.
Thanks.
Aldy
>
> OK if doing it that way.
>
> Thanks,
> Richard.
>
> > --
> > 2.37.1
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] A == 0 ? A : -A same as -A (when A is 0.0)
2022-08-30 7:06 ` Aldy Hernandez
@ 2022-08-30 7:12 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2022-08-30 7:12 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: GCC patches, MacLeod, Andrew
On Tue, Aug 30, 2022 at 9:07 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Tue, Aug 30, 2022 at 9:03 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Mon, Aug 29, 2022 at 10:45 PM Aldy Hernandez via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > The upcoming work for frange triggers a regression in
> > > gcc.dg/tree-ssa/phi-opt-24.c.
> > >
> > > For -O2 -fno-signed-zeros, we fail to transform the following into -A:
> > >
> > > float f0(float A)
> > > {
> > > // A == 0? A : -A same as -A
> > > if (A == 0) return A;
> > > return -A;
> > > }
> > >
> > > This is because the abs/negative match.pd pattern here:
> > >
> > > /* abs/negative simplifications moved from fold_cond_expr_with_comparison,
> > > Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> > > Need to handle UN* comparisons.
> > > ...
> > > ...
> > >
> > > Sees IL that has the 0.0 propagated.
> > >
> > > Instead of:
> > >
> > > <bb 2> [local count: 1073741824]:
> > > if (A_2(D) == 0.0)
> > > goto <bb 4>; [34.00%]
> > > else
> > > goto <bb 3>; [66.00%]
> > >
> > > <bb 3> [local count: 708669601]:
> > > _3 = -A_2(D);
> > >
> > > <bb 4> [local count: 1073741824]:
> > > # _1 = PHI <A_2(D)(2), _3(3)>
> > >
> > > It now sees:
> > >
> > > <bb 4> [local count: 1073741824]:
> > > # _1 = PHI <0.0(2), _3(3)>
> > >
> > > which it leaves untouched, causing the if conditional to survive.
> > >
> > > Adding a variant to the pattern that for real_zerop fixes the problem.
> > >
> > > I am a match.pd weenie, and am avoiding touching more patterns that
> > > may also benefit from handling real constants. So please use simple
> > > words if what I'm doing isn't correct ;-).
> > >
> > > I did not include a testcase, as it's just phi-opt-24.c which will get
> > > triggered when I commit the frange with endpoints work.
> > >
> > > Tested on x86-64 & ppc64le Linux.
> > >
> > > OK?
> > >
> > > gcc/ChangeLog:
> > >
> > > * match.pd ((cmp @0 zerop) real_zerop (negate@1 @0)): Add variant
> > > for real zero.
> > > ---
> > > gcc/match.pd | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index 1bb936fc401..5bdea300f94 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -4803,6 +4803,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > (cnd (cmp @0 zerop) @0 (negate@1 @0))
> > > (if (!HONOR_SIGNED_ZEROS (type))
> > > @1))
> > > + (simplify
> > > + (cnd (cmp @0 zerop) real_zerop (negate@1 @0))
> > > + (if (!HONOR_SIGNED_ZEROS (type))
> > > + @1))
> > > (simplify
> > > (cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
> > > (if (!HONOR_SIGNED_ZEROS (type))
> >
> > Looking at zerop:
> >
> > bool
> > zerop (const_tree expr)
> > {
> > return (integer_zerop (expr)
> > || real_zerop (expr)
> > || fixed_zerop (expr));
> > }
> >
> > your patch is equivalent to changing the integer_zerop one to use zerop?
> > (well, with the exception of also covering fixed_zerop).
>
> Heh. That was my first instinct, but I thought fixed_zerop didn't
> apply? No clue...I'm happy to change it.
Well, it already applies when the constant is not propagated, so ...
> Thanks.
> Aldy
>
> >
> > OK if doing it that way.
> >
> > Thanks,
> > Richard.
> >
> > > --
> > > 2.37.1
> > >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] A == 0 ? A : -A same as -A (when A is 0.0)
2022-08-29 20:44 [PATCH] A == 0 ? A : -A same as -A (when A is 0.0) Aldy Hernandez
2022-08-29 20:44 ` Aldy Hernandez
2022-08-30 7:03 ` Richard Biener
@ 2022-09-07 16:54 ` Jeff Law
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2022-09-07 16:54 UTC (permalink / raw)
To: gcc-patches
On 8/29/2022 2:44 PM, Aldy Hernandez via Gcc-patches wrote:
> The upcoming work for frange triggers a regression in
> gcc.dg/tree-ssa/phi-opt-24.c.
>
> For -O2 -fno-signed-zeros, we fail to transform the following into -A:
>
> float f0(float A)
> {
> // A == 0? A : -A same as -A
> if (A == 0) return A;
> return -A;
> }
>
> This is because the abs/negative match.pd pattern here:
>
> /* abs/negative simplifications moved from fold_cond_expr_with_comparison,
> Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> Need to handle UN* comparisons.
> ...
> ...
>
> Sees IL that has the 0.0 propagated.
>
> Instead of:
>
> <bb 2> [local count: 1073741824]:
> if (A_2(D) == 0.0)
> goto <bb 4>; [34.00%]
> else
> goto <bb 3>; [66.00%]
>
> <bb 3> [local count: 708669601]:
> _3 = -A_2(D);
>
> <bb 4> [local count: 1073741824]:
> # _1 = PHI <A_2(D)(2), _3(3)>
>
> It now sees:
>
> <bb 4> [local count: 1073741824]:
> # _1 = PHI <0.0(2), _3(3)>
>
> which it leaves untouched, causing the if conditional to survive.
>
> Adding a variant to the pattern that for real_zerop fixes the problem.
>
> I am a match.pd weenie, and am avoiding touching more patterns that
> may also benefit from handling real constants. So please use simple
> words if what I'm doing isn't correct ;-).
>
> I did not include a testcase, as it's just phi-opt-24.c which will get
> triggered when I commit the frange with endpoints work.
>
> Tested on x86-64 & ppc64le Linux.
>
> OK?
>
> gcc/ChangeLog:
>
> * match.pd ((cmp @0 zerop) real_zerop (negate@1 @0)): Add variant
> for real zero.
OK
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-07 16:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 20:44 [PATCH] A == 0 ? A : -A same as -A (when A is 0.0) Aldy Hernandez
2022-08-29 20:44 ` Aldy Hernandez
2022-08-30 7:03 ` Richard Biener
2022-08-30 7:06 ` Aldy Hernandez
2022-08-30 7:12 ` Richard Biener
2022-09-07 16:54 ` Jeff Law
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).