Re: [PATCH] [PR24021] Implement PLUS_EXPR range-op entry for floats.

[Aldy Hernandez <aldyh@redhat.com>]

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

On Tue, Nov 8, 2022 at 2:50 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Nov 08, 2022 at 02:47:35PM +0100, Aldy Hernandez wrote:
> > Well, perhaps we should just nuke update_nan_sign() altogether, and
> > always keep the sign varying?
> >
> > inline bool
> > propagate_nans (frange &r, const frange &op1, const frange &op2)
> > {
> >   if (op1.known_isnan () || op2.known_isnan ())
> >     {
> >       r.set_nan (op1.type ());
> >       return true;
> >     }
> >   return false;
> > }
> >
> > I'm fine either way.  The less code the better :).
>
> Yes, but you had 2 callers, so something needs to be done also if
> in foperator_plus::fold_range.

We can also remove the update_nan_sign() in the other call because the
r.set() before it sets a default NAN (with a varying sign).

Attached patch in testing.

Aldy

[-- Attachment #2: 0002-PR24021-Implement-PLUS_EXPR-range-op-entry-for-float.patch --]
[-- Type: text/x-patch, Size: 7022 bytes --]

From 8da4fc39cc73f0ae785463bd5f371223fa59027e Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Thu, 13 Oct 2022 08:14:16 +0200
Subject: [PATCH] [PR24021] Implement PLUS_EXPR range-op entry for floats.

This is the range-op entry for floating point PLUS_EXPR.  It's the
most intricate range entry we have so far, because we need to keep
track of rounding and target FP formats.  This will be the last FP
entry I commit, mostly to avoid disturbing the tree any further, and
also because what we have so far is enough for a solid VRP.

So far we track NANs and signs correctly.  We also handle relationals
(symbolics and numeric), both ordered and unordered, ABS_EXPR and
NEGATE_EXPR which are used to fold __builtin_isinf, and __builtin_sign
(__builtin_copysign is coming up).  All in all, I think this provide
more than enough for basic VRP on floats, as well as provide a basis
to flesh out the rest if there's interest.

My goal with this entry is to provide a template for additional binary
operators, as they tend to follow a similar pattern: handle NANs, do
the arithmetic while keeping track of rounding, and adjust for NAN.  I
may abstract the general parts as we do for irange's fold_range and
wi_fold.

	PR tree-optimization/24021

gcc/ChangeLog:

	* range-op-float.cc (propagate_nans): New.
	(frange_nextafter): New.
	(frange_arithmetic): New.
	(class foperator_plus): New.
	(floating_op_table::floating_op_table): Add PLUS_EXPR entry.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/vrp-float-plus.c: New test.
---
 gcc/range-op-float.cc                         | 128 ++++++++++++++++++
 .../gcc.dg/tree-ssa/vrp-float-plus.c          |  21 +++
 2 files changed, 149 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-plus.c

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index a1f372997bf..502b67aaa66 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -192,6 +192,81 @@ frelop_early_resolve (irange &r, tree type,
 	  && relop_early_resolve (r, type, op1, op2, rel, my_rel));
 }
 
+// If either operand is a NAN, set R to the combination of both NANs
+// signwise and return TRUE.
+
+inline bool
+propagate_nans (frange &r, const frange &op1, const frange &op2)
+{
+  if (op1.known_isnan () || op2.known_isnan ())
+    {
+      r.set_nan (op1.type ());
+      return true;
+    }
+  return false;
+}
+
+// Set VALUE to its next real value, or INF if the operation overflows.
+
+inline void
+frange_nextafter (enum machine_mode mode,
+		  REAL_VALUE_TYPE &value,
+		  const REAL_VALUE_TYPE &inf)
+{
+  const real_format *fmt = REAL_MODE_FORMAT (mode);
+  REAL_VALUE_TYPE tmp;
+  real_nextafter (&tmp, fmt, &value, &inf);
+  value = tmp;
+}
+
+// Like real_arithmetic, but round the result to INF if the operation
+// produced inexact results.
+//
+// ?? There is still one problematic case, i387.  With
+// -fexcess-precision=standard we perform most SF/DFmode arithmetic in
+// XFmode (long_double_type_node), so that case is OK.  But without
+// -mfpmath=sse, all the SF/DFmode computations are in XFmode
+// precision (64-bit mantissa) and only occassionally rounded to
+// SF/DFmode (when storing into memory from the 387 stack).  Maybe
+// this is ok as well though it is just occassionally more precise. ??
+
+static void
+frange_arithmetic (enum tree_code code, tree type,
+		   REAL_VALUE_TYPE &result,
+		   const REAL_VALUE_TYPE &op1,
+		   const REAL_VALUE_TYPE &op2,
+		   const REAL_VALUE_TYPE &inf)
+{
+  REAL_VALUE_TYPE value;
+  enum machine_mode mode = TYPE_MODE (type);
+  bool mode_composite = MODE_COMPOSITE_P (mode);
+
+  bool inexact = real_arithmetic (&value, code, &op1, &op2);
+  real_convert (&result, mode, &value);
+
+  // Be extra careful if there may be discrepancies between the
+  // compile and runtime results.
+  if ((mode_composite || (real_isneg (&inf) ? real_less (&result, &value)
+			  : !real_less (&value, &result)))
+      && (inexact || !real_identical (&result, &value)))
+    {
+      if (mode_composite)
+	{
+	  if (real_isdenormal (&result, mode)
+	      || real_iszero (&result))
+	    {
+	      // IBM extended denormals only have DFmode precision.
+	      REAL_VALUE_TYPE tmp;
+	      real_convert (&tmp, DFmode, &value);
+	      frange_nextafter (DFmode, tmp, inf);
+	      real_convert (&result, mode, &tmp);
+	      return;
+	    }
+	}
+      frange_nextafter (mode, result, inf);
+    }
+}
+
 // Crop R to [-INF, MAX] where MAX is the maximum representable number
 // for TYPE.
 
@@ -1746,6 +1821,58 @@ foperator_unordered_equal::op1_range (frange &r, tree type,
   return true;
 }
 
+class foperator_plus : public range_operator_float
+{
+  using range_operator_float::fold_range;
+
+public:
+  bool fold_range (frange &r, tree type,
+		   const frange &lh,
+		   const frange &rh,
+		   relation_trio = TRIO_VARYING) const final override;
+} fop_plus;
+
+bool
+foperator_plus::fold_range (frange &r, tree type,
+			    const frange &op1, const frange &op2,
+			    relation_trio) const
+{
+  if (empty_range_varying (r, type, op1, op2))
+    return true;
+  if (propagate_nans (r, op1, op2))
+    return true;
+
+  REAL_VALUE_TYPE lb, ub;
+  frange_arithmetic (PLUS_EXPR, type, lb,
+		     op1.lower_bound (), op2.lower_bound (), dconstninf);
+  frange_arithmetic (PLUS_EXPR, type, ub,
+		     op1.upper_bound (), op2.upper_bound (), dconstinf);
+
+  // Handle possible NANs by saturating to the appropriate INF if only
+  // one end is a NAN.  If both ends are a NAN, just return a NAN.
+  bool lb_nan = real_isnan (&lb);
+  bool ub_nan = real_isnan (&ub);
+  if (lb_nan && ub_nan)
+    {
+      r.set_nan (type);
+      return true;
+    }
+  if (lb_nan)
+    lb = dconstninf;
+  else if (ub_nan)
+    ub = dconstinf;
+
+  r.set (type, lb, ub);
+
+  if (lb_nan || ub_nan)
+    // Keep the default NAN (with a varying sign) set by the setter.
+    ;
+  else if (!op1.maybe_isnan () && !op2.maybe_isnan ())
+    r.clear_nan ();
+
+  return true;
+}
+
 // Instantiate a range_op_table for floating point operations.
 static floating_op_table global_floating_table;
 
@@ -1778,6 +1905,7 @@ floating_op_table::floating_op_table ()
 
   set (ABS_EXPR, fop_abs);
   set (NEGATE_EXPR, fop_negate);
+  set (PLUS_EXPR, fop_plus);
 }
 
 // Return a pointer to the range_operator_float instance, if there is
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-plus.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-plus.c
new file mode 100644
index 00000000000..3739ea4e810
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-plus.c
@@ -0,0 +1,21 @@
+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-fre -fno-tree-dominator-opts -fno-thread-jumps -fdump-tree-vrp2" }
+
+double BG_SplineLength ()
+{
+  double lastPoint;
+  double i;
+
+  for (i = 0.01;i<=1;i+=0.1f)
+    if (!(i != 0.0))
+      {
+        lastPoint = i;
+      }
+    else
+      {
+        lastPoint = 2;
+      }
+  return lastPoint;
+}
+
+// { dg-final { scan-tree-dump-times "return 2\\.0e" 1 "vrp2" } }
-- 
2.38.1