[COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR.

Message ID 20221109090246.1036213-1-aldyh@redhat.com
State Committed
Commit 4287e8168f89e90b3dff3a50f3ada40be53e0e01
Headers
Series [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR. |

Commit Message

Aldy Hernandez Nov. 9, 2022, 9:02 a.m. UTC
  We can implement the op[12]_range entries for plus and minus in terms
of each other.  These are adapted from the integer versions.

gcc/ChangeLog:

	* range-op-float.cc (foperator_plus::op1_range): New.
	(foperator_plus::op2_range): New.
	(foperator_minus::op1_range): New.
	(foperator_minus::op2_range): New.
---
 gcc/range-op-float.cc | 45 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
  

Comments

Jakub Jelinek Nov. 9, 2022, 2:58 p.m. UTC | #1
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
  
Aldy Hernandez Nov. 9, 2022, 3:43 p.m. UTC | #2
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
  
Jakub Jelinek Nov. 9, 2022, 3:53 p.m. UTC | #3
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
  

Patch

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,