From: "Roger Sayle" <roger@nextmovesoftware.com>
To: "'Richard Biener'" <rguenther@suse.de>,
"'Jakub Jelinek'" <jakub@redhat.com>
Cc: "'Joseph S. Myers'" <joseph@codesourcery.com>,
"'Jason Merrill'" <jason@redhat.com>, <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH] fold-const: Don't consider NaN non-negative [PR97965]
Date: Thu, 26 Nov 2020 13:56:03 -0000 [thread overview]
Message-ID: <02b501d6c3fb$e248fc90$a6daf5b0$@nextmovesoftware.com> (raw)
In-Reply-To: <nycvar.YFH.7.77.849.2011261003140.7048@jbgna.fhfr.qr>
[-- Attachment #1: Type: text/plain, Size: 2948 bytes --]
Many thanks for including me on this discussion. It's extremely
interesting...
NaNs have a sign-bit, so copyexpr (and negate and ABS_EXPR) are
well-defined,
and it's reasonable for nonnegative_p to reflect this. IMHO, the true bug
is that
we can't fold away (any) comparisons against NaN when flag_trapping_math,
irrespective of qNaN, -qNaN, sNaN or -sNaN.
My completely untested solution is the attached patch. My apologies, I'm
not
even set up to compile things on the laptop that I'm composing this e-mail
on,
but my notes/proposals on tackling PR97965 are easier expressed as the
actual
suggested changes/edits. [Forgive me if I've made a typo].
As Joseph correctly points out, signaling NaNs are a red herring here, as
both
sNaNs and qNaNs may trap during regular (ordered) comparisons.
The piece that I'll leave to the (C++) front-end folks to argue, is whether
constexpr
implies -fno-trapping-math like initializers, c.f.
START_FOLD_INIT/END_FOLD_INIT
in fold-const.c. This controls whether the test case should be consistently
true or
consistently false, but the patches above address Jakub's concern in the PR
that
things should at least be consistent.
I hope this helps. I'm happy to spin this patch myself but it may take a
little while.
Hopefully, this is sufficient to point folks in the right (or one possible)
direction.
Best regards,
Roger
--
Roger Sayle
NextMove Software Limited
Cambridge, UK
-----Original Message-----
From: Richard Biener <rguenther@suse.de>
Sent: 26 November 2020 10:04
To: Jakub Jelinek <jakub@redhat.com>
Cc: Joseph S. Myers <joseph@codesourcery.com>; Jason Merrill
<jason@redhat.com>; gcc-patches@gcc.gnu.org; roger@nextmovesoftware.com
Subject: Re: [PATCH] fold-const: Don't consider NaN non-negative [PR97965]
On Thu, 26 Nov 2020, Jakub Jelinek wrote:
> On Thu, Nov 26, 2020 at 09:16:29AM +0000, Richard Biener wrote:
> > > So, I really don't know if we want this or not, posting it for
discussions.
> >
> > Is copysign (x, NaN) supposed to be well-defined? We'd stop folding
> > this then, no?
>
> Yes, we'd stop folding several cases with NaNs.
>
> > I think the ABS_EXPR<x> < 0 to false folding is simply incomplete
> > and should first check whether the operands are ordered? That said,
> > NaN is nonnegative but NaN < 0 isn't false(?)
> >
> > So I don't think the patch is good.
>
> Another possibility (if we have this optimization already in match.pd
> too) would be to only optimize the < 0 case in GENERIC if !HONOR_NANS
> like the >= 0 case is and only optimize it in GIMPLE. Though with the
> default -ftrapping-math I think even optimizing qNaN < 0 to 0 is
> incorrect, even that should raise invalid exception, shouldn't it?
> So perhaps add a defaulted argument to the *nonnegative* APIs that
> would say whether unordered is ok or not?
Roger recently added some exhaustive changes in related areas, so let's see
if he has anything to say here.
Richard.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2377 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 632a241a964..b76e80c02a3 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -12007,8 +12007,8 @@ fold_binary_loc (location_t loc, enum tree_code code, tree type,
strict_overflow_p = false;
if (code == GE_EXPR
&& (integer_zerop (arg1)
- || (! HONOR_NANS (arg0)
- && real_zerop (arg1)))
+ || (real_zerop (arg1)
+ && !tree_expr_maybe_nan_p (arg0)))
&& tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p))
{
if (strict_overflow_p)
@@ -12024,7 +12024,10 @@ fold_binary_loc (location_t loc, enum tree_code code, tree type,
/* Convert ABS_EXPR<x> < 0 to false. */
strict_overflow_p = false;
if (code == LT_EXPR
- && (integer_zerop (arg1) || real_zerop (arg1))
+ && (integer_zerop (arg1)
+ || (real_zerop (arg1)
+ && (!flag_trapping_math
+ || !tree_expr_maybe_nan_p (arg0))))
&& tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p))
{
if (strict_overflow_p)
diff --git a/gcc/match.pd b/gcc/match.pd
index f8b65154a9e..dec19ed2d57 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3998,7 +3998,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(cmp @0 { build_real (TREE_TYPE (@1), dconst0); }))
/* x != NaN is always true, other ops are always false. */
(if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1))
- && ! HONOR_SNANS (@1))
+ && ! flag_trapping_math)
{ constant_boolean_node (cmp == NE_EXPR, type); })
/* Fold comparisons against infinity. */
(if (REAL_VALUE_ISINF (TREE_REAL_CST (@1))
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 47e7aebda8a..ab53570ac8c 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -5732,12 +5732,13 @@ simplify_const_relational_operation (enum rtx_code code,
if (REAL_VALUE_ISNAN (*d0) || REAL_VALUE_ISNAN (*d1))
switch (code)
{
+ case NE:
+ return flag_trapping_math ? 0 : const_true_rtx;
case UNEQ:
case UNLT:
case UNGT:
case UNLE:
case UNGE:
- case NE:
case UNORDERED:
return const_true_rtx;
case EQ:
@@ -5746,6 +5747,7 @@ simplify_const_relational_operation (enum rtx_code code,
case LE:
case GE:
case LTGT:
+ return flag_trapping_math ? 0 : const0_rtx;
case ORDERED:
return const0_rtx;
default:
next prev parent reply other threads:[~2020-11-26 13:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-26 8:31 Jakub Jelinek
2020-11-26 9:16 ` Richard Biener
2020-11-26 9:49 ` Jakub Jelinek
2020-11-26 10:03 ` Richard Biener
2020-11-26 13:56 ` Roger Sayle [this message]
2020-11-26 14:13 ` Jakub Jelinek
2020-11-26 14:20 ` Jakub Jelinek
2020-11-26 22:43 ` Jakub Jelinek
2020-11-27 10:51 ` Roger Sayle
2020-11-27 10:56 ` Roger Sayle
2020-11-27 20:39 ` Joseph Myers
2020-11-27 20:31 ` Joseph Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='02b501d6c3fb$e248fc90$a6daf5b0$@nextmovesoftware.com' \
--to=roger@nextmovesoftware.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jason@redhat.com \
--cc=joseph@codesourcery.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).