PR middle-end/88173: More constant folding of NaN comparisons.

Message ID 003501d7ac6b$18663ab0$4932b010$@nextmovesoftware.com
State New
Headers
Series PR middle-end/88173: More constant folding of NaN comparisons. |

Commit Message

Roger Sayle Sept. 18, 2021, 8:56 a.m. UTC
  This patch tackles PR middle-end/88173 where the order of operands in
a comparison affects constant folding.  As diagnosed by Jason Merrill,
"match.pd handles these comparisons very differently".  The history is
that the middle end, typically canonicalizes comparisons to place
constants on the right, but when a comparison contains two constants
we need to check/transform both constants, i.e. on both the left and the
right.  Hence the added lines below duplicate for @0 the same transform
applied a few lines above for @1.

Whilst preparing the testcase, I noticed that this transformation is
incorrectly disabled with -fsignaling-nans even when both operands are
known not be be signaling NaNs, so I've corrected that and added a
second test case.  Unfortunately, c-c++-common/pr57371-4.c then starts
failing, as it doesn't distinguish QNaNs (which are quiet) from SNaNs
(which signal), so this patch includes a minor tweak to the expected
behaviour for QNaNs in that existing test.

I suspect the middle-end (both at the tree-level and RTL) might need
to respect -ftrapping-math for the non-equality comparisons against
QNaN, but that's a different PR/patch [I need to double check/research
whether and why those checks may have been removed in the past].

This patch has been tested on x86_64-pc-linux-gnu with "make bootstrap"
and "make -k check" with no new failures.  Ok for mainline?


2021-09-18  Roger Sayle <roger@nextmovesoftware.com>

gcc/ChangeLog
	PR middle-end/88173
	* match.pd (cmp @0 REAL_CST@1): When @0 is also REAL_CST, apply
	the same transformations as to @1.  For comparisons against NaN,
	don't check HONOR_SNANS but confirm that neither operand is a
	signaling NaN.

gcc/testsuite/ChangeLog
	PR middle-end/88173
	* c-c++-common/pr57371-4.c: Tweak/correct test case for QNaNs.
	* g++.dg/pr88173-1.C: New test case.
	* g++.dg/pr88173-2.C: New test case.

Roger
--

/* { dg-do compile } */
/* { dg-options "-O2 -std=c++11" } */

#define big __builtin_huge_val()
#define nan __builtin_nan("")

constexpr bool b1 = big > nan;
constexpr bool b2 = nan < big;
/* { dg-do compile } */
/* { dg-options "-O2 -fsignaling-nans -std=c++11" } */

#define big __builtin_huge_val()
#define nan __builtin_nan("")

constexpr bool b1 = big > nan;
constexpr bool b2 = nan < big;
  

Comments

Jeff Law Sept. 19, 2021, 5:50 a.m. UTC | #1
On 9/18/2021 2:56 AM, Roger Sayle wrote:
> This patch tackles PR middle-end/88173 where the order of operands in
> a comparison affects constant folding.  As diagnosed by Jason Merrill,
> "match.pd handles these comparisons very differently".  The history is
> that the middle end, typically canonicalizes comparisons to place
> constants on the right, but when a comparison contains two constants
> we need to check/transform both constants, i.e. on both the left and the
> right.  Hence the added lines below duplicate for @0 the same transform
> applied a few lines above for @1.
>
> Whilst preparing the testcase, I noticed that this transformation is
> incorrectly disabled with -fsignaling-nans even when both operands are
> known not be be signaling NaNs, so I've corrected that and added a
> second test case.  Unfortunately, c-c++-common/pr57371-4.c then starts
> failing, as it doesn't distinguish QNaNs (which are quiet) from SNaNs
> (which signal), so this patch includes a minor tweak to the expected
> behaviour for QNaNs in that existing test.
>
> I suspect the middle-end (both at the tree-level and RTL) might need
> to respect -ftrapping-math for the non-equality comparisons against
> QNaN, but that's a different PR/patch [I need to double check/research
> whether and why those checks may have been removed in the past].
>
> This patch has been tested on x86_64-pc-linux-gnu with "make bootstrap"
> and "make -k check" with no new failures.  Ok for mainline?
>
>
> 2021-09-18  Roger Sayle <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
> 	PR middle-end/88173
> 	* match.pd (cmp @0 REAL_CST@1): When @0 is also REAL_CST, apply
> 	the same transformations as to @1.  For comparisons against NaN,
> 	don't check HONOR_SNANS but confirm that neither operand is a
> 	signaling NaN.
>
> gcc/testsuite/ChangeLog
> 	PR middle-end/88173
> 	* c-c++-common/pr57371-4.c: Tweak/correct test case for QNaNs.
> 	* g++.dg/pr88173-1.C: New test case.
> 	* g++.dg/pr88173-2.C: New test case.
OK
jeff
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 008f775..4a2f058 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4480,9 +4480,20 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    /* a CMP (-0) -> a CMP 0  */
    (if (REAL_VALUE_MINUS_ZERO (TREE_REAL_CST (@1)))
     (cmp @0 { build_real (TREE_TYPE (@1), dconst0); }))
+   /* (-0) CMP b -> 0 CMP b.  */
+   (if (TREE_CODE (@0) == REAL_CST
+	&& REAL_VALUE_MINUS_ZERO (TREE_REAL_CST (@0)))
+    (cmp { build_real (TREE_TYPE (@0), dconst0); } @1))
    /* x != NaN is always true, other ops are always false.  */
    (if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1))
-	&& ! HONOR_SNANS (@1))
+	&& !tree_expr_signaling_nan_p (@1)
+	&& !tree_expr_maybe_signaling_nan_p (@0))
+    { constant_boolean_node (cmp == NE_EXPR, type); })
+   /* NaN != y is always true, other ops are always false.  */
+   (if (TREE_CODE (@0) == REAL_CST
+	&& REAL_VALUE_ISNAN (TREE_REAL_CST (@0))
+        && !tree_expr_signaling_nan_p (@0)
+        && !tree_expr_signaling_nan_p (@1))
     { constant_boolean_node (cmp == NE_EXPR, type); })
    /* Fold comparisons against infinity.  */
    (if (REAL_VALUE_ISINF (TREE_REAL_CST (@1))
diff --git a/gcc/testsuite/c-c++-common/pr57371-4.c b/gcc/testsuite/c-c++-common/pr57371-4.c
index f43f7c2..d938ecd 100644
--- a/gcc/testsuite/c-c++-common/pr57371-4.c
+++ b/gcc/testsuite/c-c++-common/pr57371-4.c
@@ -13,25 +13,25 @@  void nonfinite(unsigned short x) {
   {
     volatile int nonfinite_1;
     nonfinite_1 = (float) x > QNAN;
-    /* { dg-final { scan-tree-dump "nonfinite_1 = \\(float\\)" "original" } } */
+    /* { dg-final { scan-tree-dump "nonfinite_1 = 0" "original" } } */
   }
 
   {
     volatile int nonfinite_2;
     nonfinite_2 = (float) x >= QNAN;
-    /* { dg-final { scan-tree-dump "nonfinite_2 = \\(float\\)" "original" } } */
+    /* { dg-final { scan-tree-dump "nonfinite_2 = 0" "original" } } */
   }
 
   {
     volatile int nonfinite_3;
     nonfinite_3 = (float) x < QNAN;
-    /* { dg-final { scan-tree-dump "nonfinite_3 = \\(float\\)" "original" } } */
+    /* { dg-final { scan-tree-dump "nonfinite_3 = 0" "original" } } */
   }
 
   {
     volatile int nonfinite_4;
     nonfinite_4 = (float) x <= QNAN;
-    /* { dg-final { scan-tree-dump "nonfinite_4 = \\(float\\)" "original" } } */
+    /* { dg-final { scan-tree-dump "nonfinite_4 = 0" "original" } } */
   }
 
   {