On Tue, Aug 30, 2022 at 9:07 AM Aldy Hernandez <al...@redhat.com> wrote:
>
> 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.
Well, it already applies when the constant is not propagated, so ...
> Thanks.
> Aldy
>
> >
> > OK if doing it that way.
> >
> > Thanks,
> > Richard.
> >
> > > --
> > > 2.37.1
> > >
> >
>
