| Message ID | 20221109090246.1036213-1-aldyh@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR. | expand |
On Wed, Nov 09, 2022 at 10:02:46AM +0100, Aldy Hernandez wrote: > We can implement the op[12]_range entries for plus and minus in terms > of each other. These are adapted from the integer versions. I think for NANs the op[12]_range shouldn't act this way. For the forward binary operations, we have the (maybe/known) NAN handling of one or both NAN operands resulting in VARYING sign (maybe/known) NAN result, that is the somehow the case for the reverse binary operations too, if result is (maybe/known) NAN and the other op is not NAN, op is VARYING sign (maybe/known) NAN, if other op is (maybe/known) NAN, then op is VARYING sign maybe NAN (always maybe, never known). But then for + we have the -INF + INF or vice versa into NAN, and that is something that shouldn't be considered. If result isn't NAN, then neither operand can be NAN, regardless of whether result can be +/- INF and the other op -/+ INF. Jakub
On Wed, Nov 9, 2022 at 3:58 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Nov 09, 2022 at 10:02:46AM +0100, Aldy Hernandez wrote: > > We can implement the op[12]_range entries for plus and minus in terms > > of each other. These are adapted from the integer versions. > > I think for NANs the op[12]_range shouldn't act this way. > For the forward binary operations, we have the (maybe/known) NAN handling > of one or both NAN operands resulting in VARYING sign (maybe/known) NAN > result, that is the somehow the case for the reverse binary operations too, > if result is (maybe/known) NAN and the other op is not NAN, op is > VARYING sign (maybe/known) NAN, if other op is (maybe/known) NAN, > then op is VARYING sign maybe NAN (always maybe, never known). > But then for + we have the -INF + INF or vice versa into NAN, and that > is something that shouldn't be considered. If result isn't NAN, then > neither operand can be NAN, regardless of whether result can be > +/- INF and the other op -/+ INF. Heh. I just ran into this while debugging the problem reported by Xi. We are solving NAN = op1 - VARYING, and trying to do it with op1 = NAN + VARYING, which returns op1 = NAN (incorrectly). I suppose in the above case op1 should ideally be [-INF,-INF][+INF,+INF]+-NAN, but since we can't represent that then [-INF,+INF] +-NAN, which is actually VARYING. Do you agree? I'm reverting this patch as attached, while I sort this out. Thanks. Aldy
On Wed, Nov 09, 2022 at 04:43:56PM +0100, Aldy Hernandez wrote: > Heh. I just ran into this while debugging the problem reported by Xi. > > We are solving NAN = op1 - VARYING, and trying to do it with op1 = NAN > + VARYING, which returns op1 = NAN (incorrectly). > > I suppose in the above case op1 should ideally be > [-INF,-INF][+INF,+INF]+-NAN, but since we can't represent that then > [-INF,+INF] +-NAN, which is actually VARYING. Do you agree? Yes. For implementation, perhaps just on the result of the reverse ops of whatever you do there currently (right before returning) call some function common to all binary ops that will handle the reverse op handling of NANs I've described. If lhs can't be NAN, then clear_nan on the result, if lhs is known to be NAN and other op can't be NAN, have result be known NAN (with VARYING sign), otherwise set maybe +-NAN on the result. Jakub
diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc index d52e971f84e..44db81c1c1e 100644 --- a/gcc/range-op-float.cc +++ b/gcc/range-op-float.cc @@ -1862,6 +1862,29 @@ foperator_unordered_equal::op1_range (frange &r, tree type, class foperator_plus : public range_operator_float { + using range_operator_float::op1_range; + using range_operator_float::op2_range; +public: + virtual bool op1_range (frange &r, tree type, + const frange &lhs, + const frange &op2, + relation_trio = TRIO_VARYING) const final override + { + if (lhs.undefined_p ()) + return false; + range_op_handler minus (MINUS_EXPR, type); + if (!minus) + return false; + return minus.fold_range (r, type, lhs, op2); + } + virtual bool op2_range (frange &r, tree type, + const frange &lhs, + const frange &op1, + relation_trio = TRIO_VARYING) const final override + { + return op1_range (r, type, lhs, op1); + } +private: void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan, tree type, const REAL_VALUE_TYPE &lh_lb, @@ -1886,6 +1909,28 @@ class foperator_plus : public range_operator_float class foperator_minus : public range_operator_float { + using range_operator_float::op1_range; + using range_operator_float::op2_range; +public: + virtual bool op1_range (frange &r, tree type, + const frange &lhs, + const frange &op2, + relation_trio = TRIO_VARYING) const final override + { + if (lhs.undefined_p ()) + return false; + return fop_plus.fold_range (r, type, lhs, op2); + } + virtual bool op2_range (frange &r, tree type, + const frange &lhs, + const frange &op1, + relation_trio = TRIO_VARYING) const final override + { + if (lhs.undefined_p ()) + return false; + return fold_range (r, type, op1, lhs); + } +private: void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan, tree type, const REAL_VALUE_TYPE &lh_lb,