Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Mar 15, 2022 at 9:03 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> > -----Original Message-----
> > From: Richard Biener <richard.guenther@gmail.com>
> > Sent: 15 March 2022 07:29
> > To: Roger Sayle <roger@nextmovesoftware.com>
> > Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> > Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> > comparisons.
> >
> > On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
> > <roger@nextmovesoftware.com> wrote:
> > >
> > >
> > > I've been wondering about the possible performance/missed-optimization
> > > impact of my patch for PR middle-end/98420 and similar IEEE
> > > correctness fixes that disable constant folding optimizations when worrying
> > about -0.0.
> > > In the common situation where the floating point result is used by a
> > > FP comparison, there's no distinction between +0.0 and -0.0, so some
> > > HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
> > >
> > > Consider the following interesting example:
> > >
> > > int foo(int x, double y) {
> > >     return (x * 0.0) < y;
> > > }
> > >
> > > Although we know that x (when converted to double) can't be NaN or
> > > Inf, we still worry that for negative values of x that (x * 0.0) may
> > > be -0.0 and so perform the multiplication at run-time.  But in this
> > > case, the result of the comparison (-0.0 < y) will be exactly the same
> > > as (+0.0 < y) for any y, hence the above may be safely constant folded to "0.0 <
> > y"
> > > avoiding the multiplication at run-time.
> > >
> > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> > > and make -k check with no new failures, and allows GCC to continue to
> > > optimize cases that we optimized in GCC 11 (without regard to correctness).
> > > Ok for mainline?
> >
> > Isn't that something that gimple-ssa-backprop.c is designed to handle?  I wonder
> > if you can see whether the signed zero speciality can be retrofitted there?
> > It currently tracks "sign does not matter", so possibly another state, "sign of
> > zero does not matter" could be introduced there.
>
> Two questions. Would adding tracking of "sign of zero does not matter" to
> gimple-ssa-backprop.c be suitable for stage4?

Probably not.

>  Secondly, even if gimple-ssa-backprop.c
> performed this kind of optimization, would that be a reason not to support
> these transformations in match.pd?

The only reason would be to avoid growing match.pd with lots of special
patterns for cases that should rarely matter in practice.  For example the
pattern at hand wouldn't trigger for (x * 0.0) * z < y which is why I thought
of backprop.  Yes, we do have match.pd patterns with similar issues already.

Basically when the pattern doesn't simplify the outermost expression it
is prone to such issues.

> Perhaps someone could open a missed
> optimization PR for backprop in Bugzilla, but the above patch still needs to be
> reviewed on its own merits.

There's a few other pieces in the patch (didn't look at it before), changing
HONOR_NANS and ltgt, those are OK independently.

One comment, instead of matching both

 (cmp (mult ...) @2)

and

  (cmp @2 (mult ..))

you can use :c on the 'cmp' - it will do the "right" thing (swap the
comparison code)
when matching the other way around.  That will reduce repetition.

>
> Speaking of tree-ssa passes that could be improved, I was wondering whether
> you could review my EVRP patch to fix regression PR/102950.  Pretty please?
> https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589569.html

I've left this to the ranger folks - you may want to ping Andrew here.

Richard.

> Thanks (as always),
> Roger
>
> > Thanks,
> > Richard.
> >
> > >
> > > 2022-03-14  Roger Sayle  <roger@nextmovesoftware.com>
> > >
> > > gcc/ChangeLog
> > >         * match.pd (X CMP (Y-Y) -> X CMP 0.0): New transformation.
> > >         (X CMP (Y * 0.0) -> X CMP 0.0): Likewise.
> > >         (X CMP X -> true): Test tree_expr_maybe_nan_p instead of
> > HONOR_NANS.
> > >         (X LTGT X -> false): Enable if X is not tree_expr_maybe_nan_p, as
> > >         this can't trap/signal.
> > >
> > > gcc/testsuite/ChangeLog
> > >         * gcc.dg/fold-compare-9.c: New test case.
> > >
> > >
> > > Thanks in advance,
> > > Roger
> > > --
> > >
>