On Tue, Aug 30, 2022 at 9:03 AM Richard Biener
<richard.guent...@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
> >
>
