Re: [COMMITTED] [range-op-float] Abstract out binary operator code out of PLUS_EXPR entry.
On Wed, Nov 09, 2022 at 02:32:55PM +0100, Jakub Jelinek wrote: > On Wed, Nov 09, 2022 at 02:14:19PM +0100, Aldy Hernandez wrote: > > On Wed, Nov 9, 2022 at 1:48 PM Jakub Jelinek wrote: > > > > > > On Wed, Nov 09, 2022 at 08:07:57AM +0100, Aldy Hernandez wrote: > > > > The PLUS_EXPR was always meant to be a template for further > > > > development, since most of the binary operators will share a similar > > > > structure. This patch abstracts out the common bits into the default > > > > definition for range_operator_float::fold_range() and provides an > > > > rv_fold() to be implemented by the individual entries wishing to use > > > > the generic folder. This is akin to what we do with fold_range() and > > > > wi_fold() in the integer version of range-ops. > > > > > > Shouldn't foperator_mult be very similar to this (except that until > > > division is done op[12]_range can't be implemented), with the exception > > > that the invalid case isn't -INF + INF or INF + -INF, but > > > 0 * +/-INF or +/-INF * 0? > > > > Multiplication and division are tricky because you have to keep track > > of signs to order the resulting range. It's the most annoying pattern > > we have for integers: > > Ah, you're right. > Reminds me of check_for_binary_op_overflow for multiplication. On the other side, thinking more about it, it should be easier than integral, because we don't need to deal with unsigned/wrap around and overflows aren't undefined, but infinities (though I guess we still have even for +/- the question how actually say PDP floats or ARM non-IEEE mode __fp16 behave on overflows for floats that don't support infinities, if it is saturating on HUGE_VAL*/-HUGE_VAL* or wraps around). So just do the cross products, sort them to create the final range, clear_nans on it and provide nans the normal way? Jakub
Re: [COMMITTED] [range-op-float] Abstract out binary operator code out of PLUS_EXPR entry.
On Wed, Nov 09, 2022 at 02:14:19PM +0100, Aldy Hernandez wrote: > On Wed, Nov 9, 2022 at 1:48 PM Jakub Jelinek wrote: > > > > On Wed, Nov 09, 2022 at 08:07:57AM +0100, Aldy Hernandez wrote: > > > The PLUS_EXPR was always meant to be a template for further > > > development, since most of the binary operators will share a similar > > > structure. This patch abstracts out the common bits into the default > > > definition for range_operator_float::fold_range() and provides an > > > rv_fold() to be implemented by the individual entries wishing to use > > > the generic folder. This is akin to what we do with fold_range() and > > > wi_fold() in the integer version of range-ops. > > > > Shouldn't foperator_mult be very similar to this (except that until > > division is done op[12]_range can't be implemented), with the exception > > that the invalid case isn't -INF + INF or INF + -INF, but > > 0 * +/-INF or +/-INF * 0? > > Multiplication and division are tricky because you have to keep track > of signs to order the resulting range. It's the most annoying pattern > we have for integers: Ah, you're right. Reminds me of check_for_binary_op_overflow for multiplication. Jakub
Re: [COMMITTED] [range-op-float] Abstract out binary operator code out of PLUS_EXPR entry.
On Wed, Nov 9, 2022 at 1:48 PM Jakub Jelinek wrote: > > On Wed, Nov 09, 2022 at 08:07:57AM +0100, Aldy Hernandez wrote: > > The PLUS_EXPR was always meant to be a template for further > > development, since most of the binary operators will share a similar > > structure. This patch abstracts out the common bits into the default > > definition for range_operator_float::fold_range() and provides an > > rv_fold() to be implemented by the individual entries wishing to use > > the generic folder. This is akin to what we do with fold_range() and > > wi_fold() in the integer version of range-ops. > > Shouldn't foperator_mult be very similar to this (except that until > division is done op[12]_range can't be implemented), with the exception > that the invalid case isn't -INF + INF or INF + -INF, but > 0 * +/-INF or +/-INF * 0? Multiplication and division are tricky because you have to keep track of signs to order the resulting range. It's the most annoying pattern we have for integers: // Multiplications, divisions and shifts are a bit tricky to handle, // depending on the mix of signs we have in the two ranges, we need to // operate on different values to get the minimum and maximum values // for the new range. One approach is to figure out all the // variations of range combinations and do the operations. // // However, this involves several calls to compare_values and it is // pretty convoluted. It's simpler to do the 4 operations (MIN0 OP // MIN1, MIN0 OP MAX1, MAX0 OP MIN1 and MAX0 OP MAX0 OP MAX1) and then // figure the smallest and largest values to form the new range. But if you have a simpler approach, have at it. I may have to bail on multiplication and division for this cycle, cause I'm running out of cycles :-/. Hmmm...even if we don't get to implement mult/div in this cycle, perhaps we could at least figure out if we'll NAN as you've suggested above. So, set [-INF,+INF] but without a NAN when applicable. Aldy
Re: [COMMITTED] [range-op-float] Abstract out binary operator code out of PLUS_EXPR entry.
On Wed, Nov 09, 2022 at 08:07:57AM +0100, Aldy Hernandez wrote: > The PLUS_EXPR was always meant to be a template for further > development, since most of the binary operators will share a similar > structure. This patch abstracts out the common bits into the default > definition for range_operator_float::fold_range() and provides an > rv_fold() to be implemented by the individual entries wishing to use > the generic folder. This is akin to what we do with fold_range() and > wi_fold() in the integer version of range-ops. Shouldn't foperator_mult be very similar to this (except that until division is done op[12]_range can't be implemented), with the exception that the invalid case isn't -INF + INF or INF + -INF, but 0 * +/-INF or +/-INF * 0? Jakub
[COMMITTED] [range-op-float] Abstract out binary operator code out of PLUS_EXPR entry.
The PLUS_EXPR was always meant to be a template for further
development, since most of the binary operators will share a similar
structure. This patch abstracts out the common bits into the default
definition for range_operator_float::fold_range() and provides an
rv_fold() to be implemented by the individual entries wishing to use
the generic folder. This is akin to what we do with fold_range() and
wi_fold() in the integer version of range-ops.
gcc/ChangeLog:
* range-op-float.cc (range_operator_float::fold_range): Abstract
out from foperator_plus.
(range_operator_float::rv_fold): New.
(foperator_plus::fold_range): Remove.
(foperator_plus::rv_fold): New.
(propagate_nans): Remove.
* range-op.h (class range_operator_float): Add rv_fold.
---
gcc/range-op-float.cc | 156 +-
gcc/range-op.h| 7 ++
2 files changed, 84 insertions(+), 79 deletions(-)
diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 8282c912fc4..7075c25442a 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -49,13 +49,66 @@ along with GCC; see the file COPYING3. If not see
// Default definitions for floating point operators.
bool
-range_operator_float::fold_range (frange ATTRIBUTE_UNUSED,
- tree type ATTRIBUTE_UNUSED,
- const frange ATTRIBUTE_UNUSED,
- const frange ATTRIBUTE_UNUSED,
+range_operator_float::fold_range (frange , tree type,
+ const frange , const frange ,
relation_trio) const
{
- return false;
+ if (empty_range_varying (r, type, op1, op2))
+return true;
+ if (op1.known_isnan () || op2.known_isnan ())
+{
+ r.set_nan (op1.type ());
+ return true;
+}
+
+ REAL_VALUE_TYPE lb, ub;
+ bool maybe_nan;
+ rv_fold (lb, ub, maybe_nan, type,
+ op1.lower_bound (), op1.upper_bound (),
+ op2.lower_bound (), op2.upper_bound ());
+
+ // 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 ();
+ bool ub_nan = real_isnan ();
+ 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 || maybe_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;
+}
+
+// For a given operation, fold two sets of ranges into [lb, ub].
+// MAYBE_NAN is set to TRUE if, in addition to any result in LB or
+// UB, the final range has the possiblity of a NAN.
+void
+range_operator_float::rv_fold (REAL_VALUE_TYPE ,
+ REAL_VALUE_TYPE ,
+ bool _nan,
+ tree type ATTRIBUTE_UNUSED,
+ const REAL_VALUE_TYPE _lb ATTRIBUTE_UNUSED,
+ const REAL_VALUE_TYPE _ub ATTRIBUTE_UNUSED,
+ const REAL_VALUE_TYPE _lb ATTRIBUTE_UNUSED,
+ const REAL_VALUE_TYPE _ub ATTRIBUTE_UNUSED)
+ const
+{
+ lb = dconstninf;
+ ub = dconstinf;
+ maybe_nan = true;
}
bool
@@ -192,19 +245,6 @@ frelop_early_resolve (irange , tree type,
&& relop_early_resolve (r, type, op1, op2, rel, my_rel));
}
-// If either operand is a NAN, set R to NAN and return TRUE.
-
-inline bool
-propagate_nans (frange , const frange , const frange )
-{
- 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
@@ -1822,69 +1862,27 @@ foperator_unordered_equal::op1_range (frange , tree
type,
class foperator_plus : public range_operator_float
{
- using range_operator_float::fold_range;
-
-public:
- bool fold_range (frange , tree type,
- const frange ,
- const frange ,
- relation_trio = TRIO_VARYING) const final override;
+ void rv_fold (REAL_VALUE_TYPE , REAL_VALUE_TYPE , bool _nan,
+ tree type,
+ const REAL_VALUE_TYPE _lb,
+ const REAL_VALUE_TYPE _ub,
+ const REAL_VALUE_TYPE _lb,
+ const REAL_VALUE_TYPE _ub) const final override
+ {
+frange_arithmetic (PLUS_EXPR, type, lb, lh_lb, rh_lb, dconstninf);
+frange_arithmetic (PLUS_EXPR, type, ub, lh_ub, rh_ub, dconstinf);
+
+// [-INF] + [+INF] = NAN
+if (real_isinf (_lb, true) && real_isinf (_ub, false))
+ maybe_nan = true;
+// [+INF] + [-INF] = NAN
+else if (real_isinf (_ub, false) && real_isinf (_lb, true))
+ maybe_nan = true;
+
