From e02c9d9116f243643c0daba8dbcc5d1795c827c3 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Fri, 31 Mar 2023 13:41:34 +0200 Subject: [PATCH] range-op-float, value-range: Fix up handling of UN{LT,LE,GT,GE,EQ}_EXPR and handle comparisons in get_tree_range [PR91645] When looking into PR91645, I've noticed we handle UN{LT,LE,GT,GE,EQ}_EXPR comparisons incorrectly. All those are unordered or ..., we correctly return [1, 1] if one or both operands are known NANs, and correctly ask the non-UN prefixed op to fold_range if neither operand may be NAN. But for the case where one or both operands may be NAN, we always return [0, 1]. The UN* fold_range tries to handle it by asking the non-UN prefixed fold_range and if it returns [1, 1] return that, if it returns [0, 0] or [0, 1] return [0, 1], which makes sense, because the maybe NAN means that it is the non-UN prefixed fold_range unioned with [1, 1] in case the maybe NAN is actually NAN at runtime. The problem is that the non-UN prefixed fold_range always returns [0, 1] because those fold_range implementations are like: if (op1.known_isnan () || op2.known_isnan ()) r = range_false (type); else if (!maybe_isnan (op1, op2)) { ... } else r = range_true_and_false (type); and so if maybe_isnan, they always return [0, 1]. Now, thinking about it, this is unnecessary pessimization, for the case where the ... block returns range_false (type) we actually could do it also if maybe_isnan (op1, op2), because if one or both operands are NAN, the comparison will be false, and if neither is NAN, the comparison will be also false. Will fix incrementally today. Anyway, the following patch fixes it by asking the non-UN prefixed fold_range on ranges with NAN cleared, which I think does the right thing in all cases. Another change in the patch is that range_query::get_tree_range always returned VARYING for comparisons, this patch allows to ask about those as well (they are very much like binary ops, except they take the important type from the types of the operands rather than result). Initially I've developed this patch together with changes to tree-call-cdce.cc, but those result in one regression and apparently aren't actually needed to fix this bug, the range-op-float.cc changes are enough. 2023-03-31 Jakub Jelinek PR tree-optimization/91645 * range-op-float.cc (foperator_unordered_lt::fold_range, foperator_unordered_le::fold_range, foperator_unordered_gt::fold_range, foperator_unordered_ge::fold_range, foperator_unordered_equal::fold_range): Call the ordered fold_range on ranges with cleared NaNs. * value-query.cc (range_query::get_tree_range): Handle also COMPARISON_CLASS_P trees. * gcc.target/i386/pr103559-1.c: New test. * gcc.target/i386/pr103559-2.c: New test. * gcc.target/i386/pr103559-3.c: New test. * gcc.target/i386/pr103559-4.c: New test. --- gcc/range-op-float.cc | 40 ++++++++++++++++++++++++++---- gcc/testsuite/gcc.target/i386/pr103559-1.c | 13 ++++++++++ gcc/testsuite/gcc.target/i386/pr103559-2.c | 15 +++++++++++ gcc/testsuite/gcc.target/i386/pr103559-3.c | 15 +++++++++++ gcc/testsuite/gcc.target/i386/pr103559-4.c | 13 ++++++++++ gcc/value-query.cc | 18 +++++++++----- 6 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr103559-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr103559-2.c create mode 100644 gcc/testsuite/gcc.target/i386/pr103559-3.c create mode 100644 gcc/testsuite/gcc.target/i386/pr103559-4.c diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc index d212e64..cfcb9b0 100644 --- a/gcc/range-op-float.cc +++ b/gcc/range-op-float.cc @@ -1587,7 +1587,13 @@ public: r = range_true (type); return true; } - if (!fop_lt.fold_range (r, type, op1, op2, rel)) + frange op1_no_nan = op1; + frange op2_no_nan = op2; + if (op1.maybe_isnan ()) + op1_no_nan.clear_nan (); + if (op2.maybe_isnan ()) + op2_no_nan.clear_nan (); + if (!fop_lt.fold_range (r, type, op1_no_nan, op2_no_nan, rel)) return false; // The result is the same as the ordered version when the // comparison is true or when the operands cannot be NANs. @@ -1692,7 +1698,13 @@ public: r = range_true (type); return true; } - if (!fop_le.fold_range (r, type, op1, op2, rel)) + frange op1_no_nan = op1; + frange op2_no_nan = op2; + if (op1.maybe_isnan ()) + op1_no_nan.clear_nan (); + if (op2.maybe_isnan ()) + op2_no_nan.clear_nan (); + if (!fop_le.fold_range (r, type, op1_no_nan, op2_no_nan, rel)) return false; // The result is the same as the ordered version when the // comparison is true or when the operands cannot be NANs. @@ -1793,7 +1805,13 @@ public: r = range_true (type); return true; } - if (!fop_gt.fold_range (r, type, op1, op2, rel)) + frange op1_no_nan = op1; + frange op2_no_nan = op2; + if (op1.maybe_isnan ()) + op1_no_nan.clear_nan (); + if (op2.maybe_isnan ()) + op2_no_nan.clear_nan (); + if (!fop_gt.fold_range (r, type, op1_no_nan, op2_no_nan, rel)) return false; // The result is the same as the ordered version when the // comparison is true or when the operands cannot be NANs. @@ -1898,7 +1916,13 @@ public: r = range_true (type); return true; } - if (!fop_ge.fold_range (r, type, op1, op2, rel)) + frange op1_no_nan = op1; + frange op2_no_nan = op2; + if (op1.maybe_isnan ()) + op1_no_nan.clear_nan (); + if (op2.maybe_isnan ()) + op2_no_nan.clear_nan (); + if (!fop_ge.fold_range (r, type, op1_no_nan, op2_no_nan, rel)) return false; // The result is the same as the ordered version when the // comparison is true or when the operands cannot be NANs. @@ -2002,7 +2026,13 @@ public: r = range_true (type); return true; } - if (!fop_equal.fold_range (r, type, op1, op2, rel)) + frange op1_no_nan = op1; + frange op2_no_nan = op2; + if (op1.maybe_isnan ()) + op1_no_nan.clear_nan (); + if (op2.maybe_isnan ()) + op2_no_nan.clear_nan (); + if (!fop_equal.fold_range (r, type, op1_no_nan, op2_no_nan, rel)) return false; // The result is the same as the ordered version when the // comparison is true or when the operands cannot be NANs. diff --git a/gcc/testsuite/gcc.target/i386/pr103559-1.c b/gcc/testsuite/gcc.target/i386/pr103559-1.c new file mode 100644 index 0000000..f43aba1 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103559-1.c @@ -0,0 +1,13 @@ +/* PR tree-optimization/103559 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump " = .SQRT \\\(" "optimized" } } */ +/* { dg-final { scan-tree-dump " = sqrtf \\\(" "optimized" } } */ + +float sqrtf (float); + +float +foo (float x) +{ + return sqrtf (x); +} diff --git a/gcc/testsuite/gcc.target/i386/pr103559-2.c b/gcc/testsuite/gcc.target/i386/pr103559-2.c new file mode 100644 index 0000000..304031b --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103559-2.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/103559 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump " = .SQRT \\\(" "optimized" } } */ +/* { dg-final { scan-tree-dump-not " = sqrtf \\\(" "optimized" } } */ + +float sqrtf (float); + +float +foo (float x) +{ + if (__builtin_isless (x, 0)) + __builtin_unreachable (); + return sqrtf (x); +} diff --git a/gcc/testsuite/gcc.target/i386/pr103559-3.c b/gcc/testsuite/gcc.target/i386/pr103559-3.c new file mode 100644 index 0000000..862990a --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103559-3.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/103559 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump-not " = .SQRT \\\(" "optimized" } } */ +/* { dg-final { scan-tree-dump " = sqrtf \\\(" "optimized" } } */ + +float sqrtf (float); + +float +foo (float x) +{ + if (!__builtin_isless (x, 0)) + __builtin_unreachable (); + return sqrtf (x); +} diff --git a/gcc/testsuite/gcc.target/i386/pr103559-4.c b/gcc/testsuite/gcc.target/i386/pr103559-4.c new file mode 100644 index 0000000..ec22b56 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103559-4.c @@ -0,0 +1,13 @@ +/* PR tree-optimization/103559 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump-not " = sqrtf \\\(" "optimized" } } */ + +float sqrtf (float); + +float +foo (float x) +{ + x = x * x; + return sqrtf (x); +} diff --git a/gcc/value-query.cc b/gcc/value-query.cc index e824d86..5012850 100644 --- a/gcc/value-query.cc +++ b/gcc/value-query.cc @@ -230,15 +230,21 @@ range_query::get_tree_range (vrange &r, tree expr, gimple *stmt) default: break; } - if (BINARY_CLASS_P (expr)) + if (BINARY_CLASS_P (expr) || COMPARISON_CLASS_P (expr)) { - range_op_handler op (TREE_CODE (expr), type); + tree op0 = TREE_OPERAND (expr, 0); + tree op1 = TREE_OPERAND (expr, 1); + if (COMPARISON_CLASS_P (expr) + && !Value_Range::supports_type_p (TREE_TYPE (op0))) + return false; + range_op_handler op (TREE_CODE (expr), + BINARY_CLASS_P (expr) ? type : TREE_TYPE (op0)); if (op) { - Value_Range r0 (TREE_TYPE (TREE_OPERAND (expr, 0))); - Value_Range r1 (TREE_TYPE (TREE_OPERAND (expr, 1))); - range_of_expr (r0, TREE_OPERAND (expr, 0), stmt); - range_of_expr (r1, TREE_OPERAND (expr, 1), stmt); + Value_Range r0 (TREE_TYPE (op0)); + Value_Range r1 (TREE_TYPE (op1)); + range_of_expr (r0, op0, stmt); + range_of_expr (r1, op1, stmt); if (!op.fold_range (r, type, r0, r1)) r.set_varying (type); } -- 2.7.4