[PATCH] range-op-float: Fix up multiplication and division reverse operation [PR107879]

[PATCH] range-op-float: Fix up multiplication and division reverse operation [PR107879]

[Jakub Jelinek]

Hi!

While for the normal cases it seems to be correct to implement
reverse multiplication (op1_range/op2_range) through division
with float_binary_op_range_finish, reverse division (op1_range)
through multiplication with float_binary_op_range_finish or
(op2_range) through division with float_binary_op_range_finish,
as e.g. following testcase shows for the corner cases it is
incorrect.
Say on the testcase we are doing reverse multiplication, we
have [-0., 0.] range (no NAN) on lhs and VARYING on op1 (or op2).
We implement that through division, because x from
lhs = x * op2
is
x = lhs / op2
For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
float_binary_op_range_finish, which figures out that because lhs
can't be NAN, neither operand can be NAN.  So, the end range is
[-0., 0.].  But that is not correct for the reverse multiplication.
When the result is 0, if op2 can be zero, then x can be anything
(VARYING), to be precise anything but INF (unless result can be NAN),
because anything * 0 is 0 (or NAN for INF).  While if op2 must be
non-zero, then x must be 0.  Of course the sign logic
(signbit(x) = signbit(lhs) ^ signbit(op2)) still holds, so it actually
isn't full VARYING if both lhs and op2 have known sign bits.
And going through other corner cases one by one shows other differences
between what we compute for the corresponding forward operation and
what we should compute for the reverse operations.
The following patch is slightly conservative and includes INF
(in case of result including 0 and not NAN) in the ranges or
0 in the ranges (in case of result including INF and not NAN).
The latter is what happens anyway because we flush denormals to 0,
and the former just not to deal with all the corner cases.
So, the end test is that for reverse multiplication and division
op2_range the cases we need to adjust to VARYING or VARYING positive
or VARYING negative are if lhs and op? ranges both contain 0,
or both contain some infinity, while for division op1_range the
corner case is if lhs range contains 0 and op2 range contains INF or vice
versa.  Otherwise I believe ranges from the corresponding operation
are ok, or could be slightly more conservative (e.g. for
reverse multiplication, if op? range is singleton INF and lhs
range doesn't include any INF, then x's range should be UNDEFINED or
known NAN (depending on if lhs can be NAN), while the division computes
[-0., 0.] +-NAN; or similarly if op? range is only 0 and lhs range
doesn't include 0, division would compute +INF +-NAN, or -INF +-NAN,
or (for lack of multipart franges -INF +INF +-NAN just VARYING +-NAN),
while again it is UNDEFINED or known NAN.

Oh, and I found by code inspection wrong condition for the division's
known NAN result, due to thinko it would trigger not just when
both operands are known to be 0 or both are known to be INF, but
when either both are known to be 0, or at least one is known to be INF.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-11-29  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/107879
	* range-op-float.cc (foperator_mult::op1_range): If both
	lhs and op2 ranges contain zero or both ranges contain
	some infinity, set r range to zero_to_inf_range depending on
	signbit_known_p.
	(foperator_div::op2_range): Similarly for lhs and op1 ranges.
	(foperator_div::op1_range): If lhs range contains zero and op2
	range contains some infinity or vice versa, set r range to
	zero_to_inf_range depending on signbit_known_p.
	(foperator_div::rv_fold): Fix up condition for returning known NAN.

--- gcc/range-op-float.cc.jj	2022-11-18 09:00:44.371322999 +0100
+++ gcc/range-op-float.cc	2022-11-28 19:45:50.347869350 +0100
@@ -2143,8 +2143,30 @@ public:
     range_op_handler rdiv (RDIV_EXPR, type);
     if (!rdiv)
       return false;
-    return float_binary_op_range_finish (rdiv.fold_range (r, type, lhs, op2),
-					 r, type, lhs);
+    bool ret = rdiv.fold_range (r, type, lhs, op2);
+    if (ret == false)
+      return false;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
+    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op2_lb, op2_ub))
+	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
+	    && (real_isinf (&op2_lb) || real_isinf (&op2_ub))))
+      {
+	// If both lhs and op2 could be zeros or both could be infinities,
+	// we don't know anything about op1 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    // Otherwise, if op2 is a singleton INF and lhs doesn't include INF,
+    // or if lhs must be zero and op2 doesn't include zero, it would be
+    // UNDEFINED, while rdiv.fold_range computes a zero or singleton INF
+    // range.  Those are supersets of UNDEFINED, so let's keep that way.
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2271,9 +2293,27 @@ public:
   {
     if (lhs.undefined_p ())
       return false;
-    return float_binary_op_range_finish (fop_mult.fold_range (r, type, lhs,
-							      op2),
-					 r, type, lhs);
+    bool ret = fop_mult.fold_range (r, type, lhs, op2);
+    if (!ret)
+      return ret;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
+    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub)
+	 && (real_isinf (&op2_lb) || real_isinf (&op2_ub)))
+	|| ((contains_zero_p (op2_lb, op2_ub))
+	    && (real_isinf (&lhs_lb) || real_isinf (&lhs_ub))))
+      {
+	// If both lhs could be zero and op2 infinity or vice versa,
+	// we don't know anything about op1 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2282,8 +2322,26 @@ public:
   {
     if (lhs.undefined_p ())
       return false;
-    return float_binary_op_range_finish (fold_range (r, type, op1, lhs),
-					 r, type, lhs);
+    bool ret = fold_range (r, type, op1, lhs);
+    if (!ret)
+      return ret;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op1_lb = op1.lower_bound ();
+    const REAL_VALUE_TYPE &op1_ub = op1.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op1_lb, op1_ub))
+	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
+	    && (real_isinf (&op1_lb) || real_isinf (&op1_ub))))
+      {
+	// If both lhs and op1 could be zeros or both could be infinities,
+	// we don't know anything about op2 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op1_lb, op1_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
 private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
@@ -2296,7 +2354,7 @@ private:
   {
     // +-0.0 / +-0.0 or +-INF / +-INF is a known NAN.
     if ((zero_p (lh_lb, lh_ub) && zero_p (rh_lb, rh_ub))
-	|| (singleton_inf_p (lh_lb, lh_ub) || singleton_inf_p (rh_lb, rh_ub)))
+	|| (singleton_inf_p (lh_lb, lh_ub) && singleton_inf_p (rh_lb, rh_ub)))
       {
 	real_nan (&lb, "", 0, TYPE_MODE (type));
 	ub = lb;
--- gcc/testsuite/gcc.c-torture/execute/pr107879.c.jj	2022-11-28 19:53:06.720570324 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr107879.c	2022-11-28 19:51:57.281572677 +0100
@@ -0,0 +1,25 @@
+/* PR tree-optimization/107879 */
+
+__attribute__((noipa)) static double
+foo (double *y)
+{
+  volatile int ph = 0;
+  volatile double vf = 1.0;
+  double factor = vf;
+  double x = - (double) ph * factor;
+  if (x == 0)
+    *y = 1.0;
+  else
+    *y = 1.0 / x;
+  double w = 2.0 * x / factor;
+  double omww = 1 - w;
+  return omww > 0.0 ? omww : 0.0;
+}
+
+int
+main ()
+{
+  double y = 42.0;
+  if (foo (&y) != 1.0)
+    __builtin_abort ();
+}

	Jakub

Re: [PATCH] range-op-float: Fix up multiplication and division reverse operation [PR107879]

[Aldy Hernandez]

On 11/29/22 10:43, Jakub Jelinek wrote:
> Hi!
> 
> While for the normal cases it seems to be correct to implement
> reverse multiplication (op1_range/op2_range) through division
> with float_binary_op_range_finish, reverse division (op1_range)
> through multiplication with float_binary_op_range_finish or
> (op2_range) through division with float_binary_op_range_finish,
> as e.g. following testcase shows for the corner cases it is
> incorrect.
> Say on the testcase we are doing reverse multiplication, we
> have [-0., 0.] range (no NAN) on lhs and VARYING on op1 (or op2).
> We implement that through division, because x from
> lhs = x * op2
> is
> x = lhs / op2
> For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
> as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
> 0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
> float_binary_op_range_finish, which figures out that because lhs
> can't be NAN, neither operand can be NAN.  So, the end range is
> [-0., 0.].  But that is not correct for the reverse multiplication.
> When the result is 0, if op2 can be zero, then x can be anything
> (VARYING), to be precise anything but INF (unless result can be NAN),

Not an objection, just an observation... If we know it can't be INF, 
could we drop INF from the range?  We have frange_drop_{inf,ninf} for this.

> because anything * 0 is 0 (or NAN for INF).  While if op2 must be
> non-zero, then x must be 0.  Of course the sign logic
> (signbit(x) = signbit(lhs) ^ signbit(op2)) still holds, so it actually
> isn't full VARYING if both lhs and op2 have known sign bits.
> And going through other corner cases one by one shows other differences
> between what we compute for the corresponding forward operation and
> what we should compute for the reverse operations.
> The following patch is slightly conservative and includes INF
> (in case of result including 0 and not NAN) in the ranges or
> 0 in the ranges (in case of result including INF and not NAN).
> The latter is what happens anyway because we flush denormals to 0,
> and the former just not to deal with all the corner cases.
> So, the end test is that for reverse multiplication and division
> op2_range the cases we need to adjust to VARYING or VARYING positive
> or VARYING negative are if lhs and op? ranges both contain 0,
> or both contain some infinity, while for division op1_range the
> corner case is if lhs range contains 0 and op2 range contains INF or vice
> versa.  Otherwise I believe ranges from the corresponding operation
> are ok, or could be slightly more conservative (e.g. for
> reverse multiplication, if op? range is singleton INF and lhs
> range doesn't include any INF, then x's range should be UNDEFINED or
> known NAN (depending on if lhs can be NAN), while the division computes
> [-0., 0.] +-NAN; or similarly if op? range is only 0 and lhs range
> doesn't include 0, division would compute +INF +-NAN, or -INF +-NAN,
> or (for lack of multipart franges -INF +INF +-NAN just VARYING +-NAN),
> while again it is UNDEFINED or known NAN.
> 
> Oh, and I found by code inspection wrong condition for the division's
> known NAN result, due to thinko it would trigger not just when
> both operands are known to be 0 or both are known to be INF, but
> when either both are known to be 0, or at least one is known to be INF.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-11-29  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/107879
> 	* range-op-float.cc (foperator_mult::op1_range): If both
> 	lhs and op2 ranges contain zero or both ranges contain
> 	some infinity, set r range to zero_to_inf_range depending on
> 	signbit_known_p.
> 	(foperator_div::op2_range): Similarly for lhs and op1 ranges.
> 	(foperator_div::op1_range): If lhs range contains zero and op2
> 	range contains some infinity or vice versa, set r range to
> 	zero_to_inf_range depending on signbit_known_p.
> 	(foperator_div::rv_fold): Fix up condition for returning known NAN.
> 
> --- gcc/range-op-float.cc.jj	2022-11-18 09:00:44.371322999 +0100
> +++ gcc/range-op-float.cc	2022-11-28 19:45:50.347869350 +0100
> @@ -2143,8 +2143,30 @@ public:
>       range_op_handler rdiv (RDIV_EXPR, type);
>       if (!rdiv)
>         return false;
> -    return float_binary_op_range_finish (rdiv.fold_range (r, type, lhs, op2),
> -					 r, type, lhs);
> +    bool ret = rdiv.fold_range (r, type, lhs, op2);
> +    if (ret == false)
> +      return false;
> +    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
> +    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
> +    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
> +    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
> +    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op2_lb, op2_ub))
> +	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
> +	    && (real_isinf (&op2_lb) || real_isinf (&op2_ub))))
> +      {
> +	// If both lhs and op2 could be zeros or both could be infinities,
> +	// we don't know anything about op1 except maybe for the sign
> +	// and perhaps if it can be NAN or not.
> +	REAL_VALUE_TYPE lb, ub;
> +	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
> +	zero_to_inf_range (lb, ub, signbit_known);
> +	r.set (type, lb, ub);

I assume we don't know anything about the sign of the NAN because of all 
the weird IEEE rules?

> +      }
> +    // Otherwise, if op2 is a singleton INF and lhs doesn't include INF,
> +    // or if lhs must be zero and op2 doesn't include zero, it would be
> +    // UNDEFINED, while rdiv.fold_range computes a zero or singleton INF
> +    // range.  Those are supersets of UNDEFINED, so let's keep that way.
> +    return float_binary_op_range_finish (ret, r, type, lhs);
>     }
>     virtual bool op2_range (frange &r, tree type,
>   			  const frange &lhs,
> @@ -2271,9 +2293,27 @@ public:
>     {
>       if (lhs.undefined_p ())
>         return false;
> -    return float_binary_op_range_finish (fop_mult.fold_range (r, type, lhs,
> -							      op2),
> -					 r, type, lhs);
> +    bool ret = fop_mult.fold_range (r, type, lhs, op2);
> +    if (!ret)
> +      return ret;
> +    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
> +    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
> +    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
> +    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
> +    if ((contains_zero_p (lhs_lb, lhs_ub)
> +	 && (real_isinf (&op2_lb) || real_isinf (&op2_ub)))
> +	|| ((contains_zero_p (op2_lb, op2_ub))
> +	    && (real_isinf (&lhs_lb) || real_isinf (&lhs_ub))))
> +      {
> +	// If both lhs could be zero and op2 infinity or vice versa,
> +	// we don't know anything about op1 except maybe for the sign
> +	// and perhaps if it can be NAN or not.
> +	REAL_VALUE_TYPE lb, ub;
> +	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
> +	zero_to_inf_range (lb, ub, signbit_known);
> +	r.set (type, lb, ub);
> +      }
> +    return float_binary_op_range_finish (ret, r, type, lhs);
>     }
>     virtual bool op2_range (frange &r, tree type,
>   			  const frange &lhs,
> @@ -2282,8 +2322,26 @@ public:
>     {
>       if (lhs.undefined_p ())
>         return false;
> -    return float_binary_op_range_finish (fold_range (r, type, op1, lhs),
> -					 r, type, lhs);
> +    bool ret = fold_range (r, type, op1, lhs);
> +    if (!ret)
> +      return ret;
> +    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
> +    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
> +    const REAL_VALUE_TYPE &op1_lb = op1.lower_bound ();
> +    const REAL_VALUE_TYPE &op1_ub = op1.upper_bound ();
> +    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op1_lb, op1_ub))
> +	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
> +	    && (real_isinf (&op1_lb) || real_isinf (&op1_ub))))
> +      {
> +	// If both lhs and op1 could be zeros or both could be infinities,
> +	// we don't know anything about op2 except maybe for the sign
> +	// and perhaps if it can be NAN or not.
> +	REAL_VALUE_TYPE lb, ub;
> +	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op1_lb, op1_ub);
> +	zero_to_inf_range (lb, ub, signbit_known);
> +	r.set (type, lb, ub);
> +      }
> +    return float_binary_op_range_finish (ret, r, type, lhs);
>     }
>   private:
>     void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
> @@ -2296,7 +2354,7 @@ private:
>     {
>       // +-0.0 / +-0.0 or +-INF / +-INF is a known NAN.
>       if ((zero_p (lh_lb, lh_ub) && zero_p (rh_lb, rh_ub))
> -	|| (singleton_inf_p (lh_lb, lh_ub) || singleton_inf_p (rh_lb, rh_ub)))
> +	|| (singleton_inf_p (lh_lb, lh_ub) && singleton_inf_p (rh_lb, rh_ub)))
>         {
>   	real_nan (&lb, "", 0, TYPE_MODE (type));
>   	ub = lb;
> --- gcc/testsuite/gcc.c-torture/execute/pr107879.c.jj	2022-11-28 19:53:06.720570324 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr107879.c	2022-11-28 19:51:57.281572677 +0100
> @@ -0,0 +1,25 @@
> +/* PR tree-optimization/107879 */
> +
> +__attribute__((noipa)) static double
> +foo (double *y)
> +{
> +  volatile int ph = 0;
> +  volatile double vf = 1.0;
> +  double factor = vf;
> +  double x = - (double) ph * factor;
> +  if (x == 0)
> +    *y = 1.0;
> +  else
> +    *y = 1.0 / x;
> +  double w = 2.0 * x / factor;
> +  double omww = 1 - w;
> +  return omww > 0.0 ? omww : 0.0;
> +}
> +
> +int
> +main ()
> +{
> +  double y = 42.0;
> +  if (foo (&y) != 1.0)
> +    __builtin_abort ();
> +}
> 
> 	Jakub
> 

Thanks, OK.
Aldy

Re: [PATCH] range-op-float: Fix up multiplication and division reverse operation [PR107879]

[Jakub Jelinek]

On Mon, Dec 05, 2022 at 10:20:53AM +0100, Aldy Hernandez wrote:
> > For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
> > as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
> > 0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
> > float_binary_op_range_finish, which figures out that because lhs
> > can't be NAN, neither operand can be NAN.  So, the end range is
> > [-0., 0.].  But that is not correct for the reverse multiplication.
> > When the result is 0, if op2 can be zero, then x can be anything
> > (VARYING), to be precise anything but INF (unless result can be NAN),
> 
> Not an objection, just an observation... If we know it can't be INF, could
> we drop INF from the range?  We have frange_drop_{inf,ninf} for this.

Do you mind if I try that incrementally and only if it doesn't make the
code too large/too unreadable?

> > +	// If both lhs and op2 could be zeros or both could be infinities,
> > +	// we don't know anything about op1 except maybe for the sign
> > +	// and perhaps if it can be NAN or not.
> > +	REAL_VALUE_TYPE lb, ub;
> > +	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
> > +	zero_to_inf_range (lb, ub, signbit_known);
> > +	r.set (type, lb, ub);
> 
> I assume we don't know anything about the sign of the NAN because of all the
> weird IEEE rules?

Yes, sign bit of NAN is unknown after binary operation or even in the
reverse case of binary operations.
The sign rule is for non-NAN.

"When neither the inputs nor result are NaN, the sign of a product or quotient is the exclusive OR of the
operands' signs; the sign of a sum, or of a difference x–y regarded as a sum x+(–y), differs from at most one of
the addends' signs; and the sign of the result of the roundToIntegral operations and roundToIntegralExact (see
7.3.1) is the sign of the operand. These rules shall apply even when operands or results are zero or
infinite."

	Jakub

Re: [PATCH] range-op-float: Fix up multiplication and division reverse operation [PR107879]

[Aldy Hernandez]

On 12/5/22 10:37, Jakub Jelinek wrote:
> On Mon, Dec 05, 2022 at 10:20:53AM +0100, Aldy Hernandez wrote:
>>> For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
>>> as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
>>> 0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
>>> float_binary_op_range_finish, which figures out that because lhs
>>> can't be NAN, neither operand can be NAN.  So, the end range is
>>> [-0., 0.].  But that is not correct for the reverse multiplication.
>>> When the result is 0, if op2 can be zero, then x can be anything
>>> (VARYING), to be precise anything but INF (unless result can be NAN),
>>
>> Not an objection, just an observation... If we know it can't be INF, could
>> we drop INF from the range?  We have frange_drop_{inf,ninf} for this.
> 
> Do you mind if I try that incrementally and only if it doesn't make the
> code too large/too unreadable?

Sure.  And don't feel obligated to implement it either.  range-ops is a 
never ending pit of possible optimizations.  We found that out quite 
early in the design.

If you don't get to it, could you at least add a comment?

Aldy

> 
>>> +	// If both lhs and op2 could be zeros or both could be infinities,
>>> +	// we don't know anything about op1 except maybe for the sign
>>> +	// and perhaps if it can be NAN or not.
>>> +	REAL_VALUE_TYPE lb, ub;
>>> +	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
>>> +	zero_to_inf_range (lb, ub, signbit_known);
>>> +	r.set (type, lb, ub);
>>
>> I assume we don't know anything about the sign of the NAN because of all the
>> weird IEEE rules?
> 
> Yes, sign bit of NAN is unknown after binary operation or even in the
> reverse case of binary operations.
> The sign rule is for non-NAN.
> 
> "When neither the inputs nor result are NaN, the sign of a product or quotient is the exclusive OR of the
> operands' signs; the sign of a sum, or of a difference x–y regarded as a sum x+(–y), differs from at most one of
> the addends' signs; and the sign of the result of the roundToIntegral operations and roundToIntegralExact (see
> 7.3.1) is the sign of the operand. These rules shall apply even when operands or results are zero or
> infinite."
> 
> 	Jakub
> 

[PATCH] range-op-float: Improve multiplication reverse operation

[Jakub Jelinek]

On Mon, Dec 05, 2022 at 10:54:41AM +0100, Aldy Hernandez wrote:
> > Do you mind if I try that incrementally and only if it doesn't make the
> > code too large/too unreadable?
> 
> Sure.  And don't feel obligated to implement it either.  range-ops is a
> never ending pit of possible optimizations.  We found that out quite early
> in the design.
> 
> If you don't get to it, could you at least add a comment?

So like this for multiplication op1/2_range if it passes bootstrap/regtest?
For division I'll need to go to a drawing board...

2022-12-05  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (zero_to_max_range): New function.
	(foperator_plus::op1_range): If lhs can't be INF nor NAN,
	op1 can't be INF.

--- gcc/range-op-float.cc.jj	2022-12-05 11:17:34.900573272 +0100
+++ gcc/range-op-float.cc	2022-12-05 12:32:30.286753929 +0100
@@ -2004,6 +2004,29 @@ zero_to_inf_range (REAL_VALUE_TYPE &lb,
     }
 }
 
+// Set [lb, ub] to [-MAX, -0], [-MAX, +MAX] or [+0, +MAX] depending on
+// signbit_known.
+static void
+zero_to_max_range (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, tree type,
+		   int signbit_known)
+{
+  if (signbit_known > 0)
+    {
+      lb = dconst0;
+      ub = real_max_representable (type);
+    }
+  else if (signbit_known < 0)
+    {
+      lb = real_min_representable (type);
+      ub = real_value_negate (&dconst0);
+    }
+  else
+    {
+      lb = real_min_representable (type);
+      ub = real_max_representable (type);
+    }
+}
+
 class foperator_plus : public range_operator_float
 {
   using range_operator_float::op1_range;
@@ -2159,7 +2182,14 @@ public:
 	// and perhaps if it can be NAN or not.
 	REAL_VALUE_TYPE lb, ub;
 	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
-	zero_to_inf_range (lb, ub, signbit_known);
+	// If lhs can't be +-INF nor NAN, then op1 can't be +-INF -
+	// +-INF * anything is either +-INF or NAN (if op2 is +-0 or NAN).
+	if (!real_isinf (&lhs_lb)
+	    && !real_isinf (&lhs_ub)
+	    && !lhs.maybe_isnan ())
+	  zero_to_max_range (lb, ub, type, signbit_known);
+	else
+	  zero_to_inf_range (lb, ub, signbit_known);
 	r.set (type, lb, ub);
       }
     // Otherwise, if op2 is a singleton INF and lhs doesn't include INF,


	Jakub

Re: [PATCH] range-op-float: Improve multiplication reverse operation

[Aldy Hernandez]

On 12/5/22 12:59, Jakub Jelinek wrote:
> On Mon, Dec 05, 2022 at 10:54:41AM +0100, Aldy Hernandez wrote:
>>> Do you mind if I try that incrementally and only if it doesn't make the
>>> code too large/too unreadable?
>>
>> Sure.  And don't feel obligated to implement it either.  range-ops is a
>> never ending pit of possible optimizations.  We found that out quite early
>> in the design.
>>
>> If you don't get to it, could you at least add a comment?
> 
> So like this for multiplication op1/2_range if it passes bootstrap/regtest?
> For division I'll need to go to a drawing board...

Sure, looks good to me.

> 
> 2022-12-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (zero_to_max_range): New function.
> 	(foperator_plus::op1_range): If lhs can't be INF nor NAN,
> 	op1 can't be INF.
> 
> --- gcc/range-op-float.cc.jj	2022-12-05 11:17:34.900573272 +0100
> +++ gcc/range-op-float.cc	2022-12-05 12:32:30.286753929 +0100
> @@ -2004,6 +2004,29 @@ zero_to_inf_range (REAL_VALUE_TYPE &lb,
>       }
>   }
>   
> +// Set [lb, ub] to [-MAX, -0], [-MAX, +MAX] or [+0, +MAX] depending on
> +// signbit_known.
> +static void
> +zero_to_max_range (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, tree type,
> +		   int signbit_known)

My apologies for having to passs around [lb, ub] everywhere.  At least 
for rv_fold(), I'd like to rewrite it next year as:

void rv_fold (frange &r, tree type, ...);

...instead of having to pass LB, UB, and maybe_nan as references.  All 
the information in LB, UB, and maybe_nan can fit nicely in an frange. 
This is what we do in irange (wi_fold).  It also sets us up nicely for 
when we have subranges, and we can union the result.

I'll clean it up early next release.
Aldy

> +{
> +  if (signbit_known > 0)
> +    {
> +      lb = dconst0;
> +      ub = real_max_representable (type);
> +    }
> +  else if (signbit_known < 0)
> +    {
> +      lb = real_min_representable (type);
> +      ub = real_value_negate (&dconst0);
> +    }
> +  else
> +    {
> +      lb = real_min_representable (type);
> +      ub = real_max_representable (type);
> +    }
> +}
> +
>   class foperator_plus : public range_operator_float
>   {
>     using range_operator_float::op1_range;
> @@ -2159,7 +2182,14 @@ public:
>   	// and perhaps if it can be NAN or not.
>   	REAL_VALUE_TYPE lb, ub;
>   	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
> -	zero_to_inf_range (lb, ub, signbit_known);
> +	// If lhs can't be +-INF nor NAN, then op1 can't be +-INF -
> +	// +-INF * anything is either +-INF or NAN (if op2 is +-0 or NAN).
> +	if (!real_isinf (&lhs_lb)
> +	    && !real_isinf (&lhs_ub)
> +	    && !lhs.maybe_isnan ())
> +	  zero_to_max_range (lb, ub, type, signbit_known);
> +	else
> +	  zero_to_inf_range (lb, ub, signbit_known);
>   	r.set (type, lb, ub);
>         }
>       // Otherwise, if op2 is a singleton INF and lhs doesn't include INF,
> 
> 
> 	Jakub
> 

[PATCH] range-op-float: Improve binary reverse operations

[Jakub Jelinek]

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

Hi!

On Mon, Dec 05, 2022 at 02:29:36PM +0100, Aldy Hernandez wrote:
> > So like this for multiplication op1/2_range if it passes bootstrap/regtest?
> > For division I'll need to go to a drawing board...
> 
> Sure, looks good to me.

Ulrich just filed PR107972, so in the light of that PR the following patch
attempts to do that differently.

Is this also ok if it passes bootstrap/regtest?

As for testcase, I've tried both attached testcases, but unfortunately it
seems that in neither of the cases we actually figure out that res range
is finite (or for last function non-zero ordered).  So there is further
work needed on that.

2022-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/107972
	* range-op-float.cc (frange_drop_infs): New function.
	(float_binary_op_range_finish): Add OP argument.  If OP is not
	RDIV_EXPR and lhs is finite, r must be finite too.
	(foperator_plus::op1_range, foperator_minus::op1_range,
	foperator_minus::op2_range, foperator_mult::op1_range): Pass
	operation code to float_binary_op_range_finish.
	(foperator_div::op1_range): Pass RDIV_EXPR to
	float_binary_op_range_finish.  If lhs is finite, r must be finite
	too.
	(foperator_div::op2_range): Pass RDIV_EXPR to
	float_binary_op_range_finish.  If lhs is not NAN nor zero, r must
	be finite.

--- gcc/range-op-float.cc.jj	2022-12-05 11:17:34.900573272 +0100
+++ gcc/range-op-float.cc	2022-12-05 16:13:54.414845672 +0100
@@ -330,6 +330,18 @@ frange_drop_ninf (frange &r, tree type)
   r.intersect (tmp);
 }
 
+// Crop R to [MIN, MAX] where MAX is the maximum representable number
+// for TYPE and MIN the minimum representable number for TYPE.
+
+static inline void
+frange_drop_infs (frange &r, tree type)
+{
+  REAL_VALUE_TYPE max = real_max_representable (type);
+  REAL_VALUE_TYPE min = real_min_representable (type);
+  frange tmp (type, min, max);
+  r.intersect (tmp);
+}
+
 // If zero is in R, make sure both -0.0 and +0.0 are in the range.
 
 static inline void
@@ -1883,7 +1895,7 @@ foperator_unordered_equal::op1_range (fr
 
 static bool
 float_binary_op_range_finish (bool ret, frange &r, tree type,
-			      const frange &lhs)
+			      const frange &lhs, enum tree_code op)
 {
   if (!ret)
     return false;
@@ -1904,7 +1916,16 @@ float_binary_op_range_finish (bool ret,
   // If lhs isn't NAN, then neither operand could be NAN,
   // even if the reverse operation does introduce a maybe_nan.
   if (!lhs.maybe_isnan ())
-    r.clear_nan ();
+    {
+      r.clear_nan ();
+      if (op != RDIV_EXPR
+	  && !real_isinf (&lhs.lower_bound ())
+	  && !real_isinf (&lhs.upper_bound ()))
+	// For reverse + or - or *, if result is finite, then r must
+	// be finite too, as X + INF or X - INF or X * INF is always
+	// +-INF or NAN.  For division handle it in the caller.
+	frange_drop_infs (r, type);
+    }
   // If lhs is a maybe or known NAN, the operand could be
   // NAN.
   else
@@ -2020,7 +2041,7 @@ public:
     if (!minus)
       return false;
     return float_binary_op_range_finish (minus.fold_range (r, type, lhs, op2),
-					 r, type, lhs);
+					 r, type, lhs, PLUS_EXPR);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2067,7 +2088,7 @@ public:
       return false;
     return float_binary_op_range_finish (fop_plus.fold_range (r, type, lhs,
 							      op2),
-					 r, type, lhs);
+					 r, type, lhs, MINUS_EXPR);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2077,7 +2098,7 @@ public:
     if (lhs.undefined_p ())
       return false;
     return float_binary_op_range_finish (fold_range (r, type, op1, lhs),
-					 r, type, lhs);
+					 r, type, lhs, MINUS_EXPR);
   }
 private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
@@ -2166,7 +2187,7 @@ public:
     // or if lhs must be zero and op2 doesn't include zero, it would be
     // UNDEFINED, while rdiv.fold_range computes a zero or singleton INF
     // range.  Those are supersets of UNDEFINED, so let's keep that way.
-    return float_binary_op_range_finish (ret, r, type, lhs);
+    return float_binary_op_range_finish (ret, r, type, lhs, MULT_EXPR);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2313,7 +2334,12 @@ public:
 	zero_to_inf_range (lb, ub, signbit_known);
 	r.set (type, lb, ub);
       }
-    return float_binary_op_range_finish (ret, r, type, lhs);
+    // If lhs must be finite (can't be +-INF nor NAN), then op1 must be
+    // finite too - +-INF / anything is either +-INF or NAN (NAN if op2 is
+    // +-INF or NAN).
+    if (!real_isinf (&lhs_lb) && !real_isinf (&lhs_ub) && !lhs.maybe_isnan ())
+      frange_drop_infs (r, type);
+    return float_binary_op_range_finish (ret, r, type, lhs, RDIV_EXPR);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2341,7 +2367,11 @@ public:
 	zero_to_inf_range (lb, ub, signbit_known);
 	r.set (type, lb, ub);
       }
-    return float_binary_op_range_finish (ret, r, type, lhs);
+    // If lhs can't be zero nor NAN, then op2 must be finite - anything / +-INF
+    // is either +-0 or NAN (NAN if op1 is +-INF or NAN).
+    if (!contains_zero_p (lhs_lb, lhs_ub) && !lhs.maybe_isnan ())
+      frange_drop_infs (r, type);
+    return float_binary_op_range_finish (ret, r, type, lhs, RDIV_EXPR);
   }
 private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,


	Jakub

[-- Attachment #2: pr107972.c --]
[-- Type: text/plain, Size: 1075 bytes --]

double
foo (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a + b;
  if (!__builtin_isfinite (res))
    __builtin_unreachable ();
  return res;
}

double
bar (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a - b;
  if (!__builtin_isfinite (res))
    __builtin_unreachable ();
  return res;
}

double
baz (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a * b;
  if (!__builtin_isfinite (res))
    __builtin_unreachable ();
  return res;
}

double
qux (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  double res = a / b;
  if (!__builtin_isfinite (res))
    __builtin_unreachable ();
  return res;
}

double
quux (double a, double b)
{
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a  / b;
  if (__builtin_isnan (res) || res == 0.0)
    __builtin_unreachable ();
  return res;
}

[-- Attachment #3: pr107972-2.c --]
[-- Type: text/plain, Size: 1032 bytes --]

double
foo (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a + b;
  __attribute__((assume (__builtin_isfinite (res))));
  return res;
}

double
bar (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a - b;
  __attribute__((assume (__builtin_isfinite (res))));
  return res;
}

double
baz (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a * b;
  __attribute__((assume (__builtin_isfinite (res))));
  return res;
}

double
qux (double a, double b)
{
  if (!__builtin_isfinite (a))
    return 42.0;
  double res = a / b;
  __attribute__((assume (__builtin_isfinite (res))));
  return res;
}

double
quux (double a, double b)
{
  if (!__builtin_isfinite (b))
    return 42.0;
  double res = a  / b;
  __attribute__((assume (!__builtin_isnan (res) && res != 0.0)));
  return res;
}

Re: [PATCH] range-op-float: Improve binary reverse operations

[Andrew MacLeod]

On 12/5/22 10:33, Jakub Jelinek wrote:
> Hi!
>
> On Mon, Dec 05, 2022 at 02:29:36PM +0100, Aldy Hernandez wrote:
>>> So like this for multiplication op1/2_range if it passes bootstrap/regtest?
>>> For division I'll need to go to a drawing board...
>> Sure, looks good to me.
> Ulrich just filed PR107972, so in the light of that PR the following patch
> attempts to do that differently.
>
> Is this also ok if it passes bootstrap/regtest?


Id actually prefer to avoid passing the tree code around... we're trying 
to avoid that sort of thing even though Aldy temporarily introduced them 
to range-ops. Hes suppose to remove that next stage 1 :-P   Ideally 
anything "special" is locally contained to the specific routine.

It looks like the only time we actually do anything different is for 
divide op2_range?   the divide op1_range seems to still call 
frange_drop_infs() under the same conditions..

In which case, maybe we can just change op2_range to contain all the 
special casing.. frange_drop_infs doesnt seem overly complicated, so 
just specialize it?  ie for divide op2_range do something like this 
instead of the call?:

if (!ret)
   return false;
if (r.known_isnan () || lhs.known_isnan () || r.undefined_p ())
   r.set_varying (type);
else if (!lhs.maybe_isnan ())
   {
     r.clear_nan();
     if (!contains_zero_p (lhs_lb, lhs_ub))
       frange_drop_infs (r, type);
   }
else
   r.update_nan ();
return true;


or whatever the sequence precisely works out to.

Andrew


> As for testcase, I've tried both attached testcases, but unfortunately it
> seems that in neither of the cases we actually figure out that res range
> is finite (or for last function non-zero ordered).  So there is further
> work needed on that.
>
> 2022-12-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/107972
> 	* range-op-float.cc (frange_drop_infs): New function.
> 	(float_binary_op_range_finish): Add OP argument.  If OP is not
> 	RDIV_EXPR and lhs is finite, r must be finite too.
> 	(foperator_plus::op1_range, foperator_minus::op1_range,
> 	foperator_minus::op2_range, foperator_mult::op1_range): Pass
> 	operation code to float_binary_op_range_finish.
> 	(foperator_div::op1_range): Pass RDIV_EXPR to
> 	float_binary_op_range_finish.  If lhs is finite, r must be finite
> 	too.
> 	(foperator_div::op2_range): Pass RDIV_EXPR to
> 	float_binary_op_range_finish.  If lhs is not NAN nor zero, r must
> 	be finite.
>
> --- gcc/range-op-float.cc.jj	2022-12-05 11:17:34.900573272 +0100
> +++ gcc/range-op-float.cc	2022-12-05 16:13:54.414845672 +0100
> @@ -330,6 +330,18 @@ frange_drop_ninf (frange &r, tree type)
>     r.intersect (tmp);
>   }
>   
> +// Crop R to [MIN, MAX] where MAX is the maximum representable number
> +// for TYPE and MIN the minimum representable number for TYPE.
> +
> +static inline void
> +frange_drop_infs (frange &r, tree type)
> +{
> +  REAL_VALUE_TYPE max = real_max_representable (type);
> +  REAL_VALUE_TYPE min = real_min_representable (type);
> +  frange tmp (type, min, max);
> +  r.intersect (tmp);
> +}
> +
>   // If zero is in R, make sure both -0.0 and +0.0 are in the range.
>   
>   static inline void
> @@ -1883,7 +1895,7 @@ foperator_unordered_equal::op1_range (fr
>   
>   static bool
>   float_binary_op_range_finish (bool ret, frange &r, tree type,
> -			      const frange &lhs)
> +			      const frange &lhs, enum tree_code op)
>   {
>     if (!ret)
>       return false;
> @@ -1904,7 +1916,16 @@ float_binary_op_range_finish (bool ret,
>     // If lhs isn't NAN, then neither operand could be NAN,
>     // even if the reverse operation does introduce a maybe_nan.
>     if (!lhs.maybe_isnan ())
> -    r.clear_nan ();
> +    {
> +      r.clear_nan ();
> +      if (op != RDIV_EXPR
> +	  && !real_isinf (&lhs.lower_bound ())
> +	  && !real_isinf (&lhs.upper_bound ()))
> +	// For reverse + or - or *, if result is finite, then r must
> +	// be finite too, as X + INF or X - INF or X * INF is always
> +	// +-INF or NAN.  For division handle it in the caller.
> +	frange_drop_infs (r, type);
> +    }
>     // If lhs is a maybe or known NAN, the operand could be
>     // NAN.
>     else
> @@ -2020,7 +2041,7 @@ public:
>       if (!minus)
>         return false;
>       return float_binary_op_range_finish (minus.fold_range (r, type, lhs, op2),
> -					 r, type, lhs);
> +					 r, type, lhs, PLUS_EXPR);
>     }
>     virtual bool op2_range (frange &r, tree type,
>   			  const frange &lhs,
> @@ -2067,7 +2088,7 @@ public:
>         return false;
>       return float_binary_op_range_finish (fop_plus.fold_range (r, type, lhs,
>   							      op2),
> -					 r, type, lhs);
> +					 r, type, lhs, MINUS_EXPR);
>     }
>     virtual bool op2_range (frange &r, tree type,
>   			  const frange &lhs,
> @@ -2077,7 +2098,7 @@ public:
>       if (lhs.undefined_p ())
>         return false;
>       return float_binary_op_range_finish (fold_range (r, type, op1, lhs),
> -					 r, type, lhs);
> +					 r, type, lhs, MINUS_EXPR);
>     }
>   private:
>     void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
> @@ -2166,7 +2187,7 @@ public:
>       // or if lhs must be zero and op2 doesn't include zero, it would be
>       // UNDEFINED, while rdiv.fold_range computes a zero or singleton INF
>       // range.  Those are supersets of UNDEFINED, so let's keep that way.
> -    return float_binary_op_range_finish (ret, r, type, lhs);
> +    return float_binary_op_range_finish (ret, r, type, lhs, MULT_EXPR);
>     }
>     virtual bool op2_range (frange &r, tree type,
>   			  const frange &lhs,
> @@ -2313,7 +2334,12 @@ public:
>   	zero_to_inf_range (lb, ub, signbit_known);
>   	r.set (type, lb, ub);
>         }
> -    return float_binary_op_range_finish (ret, r, type, lhs);
> +    // If lhs must be finite (can't be +-INF nor NAN), then op1 must be
> +    // finite too - +-INF / anything is either +-INF or NAN (NAN if op2 is
> +    // +-INF or NAN).
> +    if (!real_isinf (&lhs_lb) && !real_isinf (&lhs_ub) && !lhs.maybe_isnan ())
> +      frange_drop_infs (r, type);
> +    return float_binary_op_range_finish (ret, r, type, lhs, RDIV_EXPR);
>     }
>     virtual bool op2_range (frange &r, tree type,
>   			  const frange &lhs,
> @@ -2341,7 +2367,11 @@ public:
>   	zero_to_inf_range (lb, ub, signbit_known);
>   	r.set (type, lb, ub);
>         }
> -    return float_binary_op_range_finish (ret, r, type, lhs);
> +    // If lhs can't be zero nor NAN, then op2 must be finite - anything / +-INF
> +    // is either +-0 or NAN (NAN if op1 is +-INF or NAN).
> +    if (!contains_zero_p (lhs_lb, lhs_ub) && !lhs.maybe_isnan ())
> +      frange_drop_infs (r, type);
> +    return float_binary_op_range_finish (ret, r, type, lhs, RDIV_EXPR);
>     }
>   private:
>     void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
>
>
> 	Jakub

Re: [PATCH] range-op-float: Improve binary reverse operations

[Jakub Jelinek]

On Mon, Dec 05, 2022 at 03:43:16PM -0500, Andrew MacLeod wrote:
> Id actually prefer to avoid passing the tree code around... we're trying to
> avoid that sort of thing even though Aldy temporarily introduced them to
> range-ops. Hes suppose to remove that next stage 1 :-P   Ideally anything
> "special" is locally contained to the specific routine.

Would a bool divide_op2 argument be better (perhaps defaulted to false)?
Inlining float_binary_op_range_finish by hand doesn't seem to be a good
idea, if it needs to be changed, it would need to be changed in multiple
places...

	Jakub

Re: [PATCH] range-op-float: Improve binary reverse operations

[Jakub Jelinek]

On Mon, Dec 05, 2022 at 09:54:09PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Mon, Dec 05, 2022 at 03:43:16PM -0500, Andrew MacLeod wrote:
> > Id actually prefer to avoid passing the tree code around... we're trying to
> > avoid that sort of thing even though Aldy temporarily introduced them to
> > range-ops. Hes suppose to remove that next stage 1 :-P   Ideally anything
> > "special" is locally contained to the specific routine.
> 
> Would a bool divide_op2 argument be better (perhaps defaulted to false)?
> Inlining float_binary_op_range_finish by hand doesn't seem to be a good
> idea, if it needs to be changed, it would need to be changed in multiple
> places...

In patch form on top of PR107975 patch.

2022-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/107972
	* range-op-float.cc (frange_drop_infs): New function.
	(float_binary_op_range_finish): Add DIV_OP2 argument.  If DIV_OP2 is
	false and lhs is finite or if DIV_OP2 is true and lhs is non-zero and
	not NAN, r must be finite too.
	(foperator_div::op2_range): Pass true to DIV_OP2 of
	float_binary_op_range_finish.

--- gcc/range-op-float.cc.jj	2022-12-05 11:17:34.900573272 +0100
+++ gcc/range-op-float.cc	2022-12-05 16:13:54.414845672 +0100
@@ -330,6 +330,18 @@ frange_drop_ninf (frange &r, tree type)
   r.intersect (tmp);
 }
 
+// Crop R to [MIN, MAX] where MAX is the maximum representable number
+// for TYPE and MIN the minimum representable number for TYPE.
+
+static inline void
+frange_drop_infs (frange &r, tree type)
+{
+  REAL_VALUE_TYPE max = real_max_representable (type);
+  REAL_VALUE_TYPE min = real_min_representable (type);
+  frange tmp (type, min, max);
+  r.intersect (tmp);
+}
+
 // If zero is in R, make sure both -0.0 and +0.0 are in the range.
 
 static inline void
@@ -1883,7 +1895,7 @@ foperator_unordered_equal::op1_range (fr
 
 static bool
 float_binary_op_range_finish (bool ret, frange &r, tree type,
-			      const frange &lhs)
+			      const frange &lhs, bool div_op2 = false)
 {
   if (!ret)
     return false;
@@ -1904,7 +1916,20 @@ float_binary_op_range_finish (bool ret,
   // If lhs isn't NAN, then neither operand could be NAN,
   // even if the reverse operation does introduce a maybe_nan.
   if (!lhs.maybe_isnan ())
-    r.clear_nan ();
+    {
+      r.clear_nan ();
+      if (div_op2
+	  ? !(real_compare (LE_EXPR, &lhs.lower_bound (), &dconst0)
+	      && real_compare (GE_EXPR, &lhs.upper_bound (), &dconst0))
+	  : !(real_isinf (&lhs.lower_bound ())
+	      || real_isinf (&lhs.upper_bound ())))
+	// For reverse + or - or * or op1 of /, if result is finite, then
+	// r must be finite too, as X + INF or X - INF or X * INF or
+	// INF / X is always +-INF or NAN.  For op2 of /, if result is
+	// non-zero and not NAN, r must be finite, as X / INF is always
+	// 0 or NAN.
+	frange_drop_infs (r, type);
+    }
   // If lhs is a maybe or known NAN, the operand could be
   // NAN.
   else
@@ -2330,7 +2355,7 @@ public:
     if (!ret)
       return ret;
     if (lhs.known_isnan () || op1.known_isnan () || op1.undefined_p ())
-      return float_binary_op_range_finish (ret, r, type, lhs);
+      return float_binary_op_range_finish (ret, r, type, lhs, true);
     const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
     const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
     const REAL_VALUE_TYPE &op1_lb = op1.lower_bound ();
@@ -2347,7 +2372,7 @@ public:
 	zero_to_inf_range (lb, ub, signbit_known);
 	r.set (type, lb, ub);
       }
-    return float_binary_op_range_finish (ret, r, type, lhs);
+    return float_binary_op_range_finish (ret, r, type, lhs, true);
   }
 private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,


	Jakub

Re: [PATCH] range-op-float: Improve binary reverse operations

[Andrew MacLeod]

On 12/5/22 17:38, Jakub Jelinek wrote:
> On Mon, Dec 05, 2022 at 09:54:09PM +0100, Jakub Jelinek via Gcc-patches wrote:
>> On Mon, Dec 05, 2022 at 03:43:16PM -0500, Andrew MacLeod wrote:
>>> Id actually prefer to avoid passing the tree code around... we're trying to
>>> avoid that sort of thing even though Aldy temporarily introduced them to
>>> range-ops. Hes suppose to remove that next stage 1 :-P   Ideally anything
>>> "special" is locally contained to the specific routine.
>> Would a bool divide_op2 argument be better (perhaps defaulted to false)?
>> Inlining float_binary_op_range_finish by hand doesn't seem to be a good
>> idea, if it needs to be changed, it would need to be changed in multiple
>> places...

Yeah thats also fine.

Andrew


> In patch form on top of PR107975 patch.
>
> 2022-12-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/107972
> 	* range-op-float.cc (frange_drop_infs): New function.
> 	(float_binary_op_range_finish): Add DIV_OP2 argument.  If DIV_OP2 is
> 	false and lhs is finite or if DIV_OP2 is true and lhs is non-zero and
> 	not NAN, r must be finite too.
> 	(foperator_div::op2_range): Pass true to DIV_OP2 of
> 	float_binary_op_range_finish.
>
> --- gcc/range-op-float.cc.jj	2022-12-05 11:17:34.900573272 +0100
> +++ gcc/range-op-float.cc	2022-12-05 16:13:54.414845672 +0100
> @@ -330,6 +330,18 @@ frange_drop_ninf (frange &r, tree type)
>     r.intersect (tmp);
>   }
>   
> +// Crop R to [MIN, MAX] where MAX is the maximum representable number
> +// for TYPE and MIN the minimum representable number for TYPE.
> +
> +static inline void
> +frange_drop_infs (frange &r, tree type)
> +{
> +  REAL_VALUE_TYPE max = real_max_representable (type);
> +  REAL_VALUE_TYPE min = real_min_representable (type);
> +  frange tmp (type, min, max);
> +  r.intersect (tmp);
> +}
> +
>   // If zero is in R, make sure both -0.0 and +0.0 are in the range.
>   
>   static inline void
> @@ -1883,7 +1895,7 @@ foperator_unordered_equal::op1_range (fr
>   
>   static bool
>   float_binary_op_range_finish (bool ret, frange &r, tree type,
> -			      const frange &lhs)
> +			      const frange &lhs, bool div_op2 = false)
>   {
>     if (!ret)
>       return false;
> @@ -1904,7 +1916,20 @@ float_binary_op_range_finish (bool ret,
>     // If lhs isn't NAN, then neither operand could be NAN,
>     // even if the reverse operation does introduce a maybe_nan.
>     if (!lhs.maybe_isnan ())
> -    r.clear_nan ();
> +    {
> +      r.clear_nan ();
> +      if (div_op2
> +	  ? !(real_compare (LE_EXPR, &lhs.lower_bound (), &dconst0)
> +	      && real_compare (GE_EXPR, &lhs.upper_bound (), &dconst0))
> +	  : !(real_isinf (&lhs.lower_bound ())
> +	      || real_isinf (&lhs.upper_bound ())))
> +	// For reverse + or - or * or op1 of /, if result is finite, then
> +	// r must be finite too, as X + INF or X - INF or X * INF or
> +	// INF / X is always +-INF or NAN.  For op2 of /, if result is
> +	// non-zero and not NAN, r must be finite, as X / INF is always
> +	// 0 or NAN.
> +	frange_drop_infs (r, type);
> +    }
>     // If lhs is a maybe or known NAN, the operand could be
>     // NAN.
>     else
> @@ -2330,7 +2355,7 @@ public:
>       if (!ret)
>         return ret;
>       if (lhs.known_isnan () || op1.known_isnan () || op1.undefined_p ())
> -      return float_binary_op_range_finish (ret, r, type, lhs);
> +      return float_binary_op_range_finish (ret, r, type, lhs, true);
>       const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
>       const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
>       const REAL_VALUE_TYPE &op1_lb = op1.lower_bound ();
> @@ -2347,7 +2372,7 @@ public:
>   	zero_to_inf_range (lb, ub, signbit_known);
>   	r.set (type, lb, ub);
>         }
> -    return float_binary_op_range_finish (ret, r, type, lhs);
> +    return float_binary_op_range_finish (ret, r, type, lhs, true);
>     }
>   private:
>     void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
>
>
> 	Jakub
>