Re: [PATCH] Add support for floating point endpoints to frange.

[Aldy Hernandez <aldyh@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]

On Fri, Aug 26, 2022, 19:40 Andrew Pinski <pinskia@gmail.com> wrote:

> On Fri, Aug 26, 2022 at 8:55 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > [pinskia: I'm CCing you as the author of the match.pd pattern.]
> >
> > So, as I wrap up the work here (latest patch attached), I see there's
> > another phiopt regression (not spaceship related).  I was hoping
> > someone could either give me a hand, or offer some guidance.
> >
> > The failure is in gcc.dg/tree-ssa/phi-opt-24.c.
> >
> > We fail to transform the following into -A:
> >
> > /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */
> >
> > 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.
> >
> > Is this something that can be done by improving the match.pd pattern,
> > or should be done elsewhere?
>
> Oh the pattern which is supposed to catch this does:
>   (simplify
>    (cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
>     (if (!HONOR_SIGNED_ZEROS (type))
>      @1))
>
> Notice the integer_zerop here.
> fold_cond_expr_with_comparison has integer_zerop there too.
> I am not 100% sure you can replace A_2 with 0.0 where you are doing it
> as mentioned in another thread.
>

Are you sure we can't make the replacement, cause the test runs with
-fno-signed-zeros?

Aldy


> Thanks,
> Andrew Pinski
>
>
> >
> > Thanks.
> > Aldy
>
>