public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR.
@ 2022-11-09  9:02 Aldy Hernandez
  2022-11-09 14:58 ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2022-11-09  9:02 UTC (permalink / raw)
  To: GCC patches; +Cc: Jakub Jelinek, Andrew MacLeod, Aldy Hernandez

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(+)

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,
-- 
2.38.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR.
  2022-11-09  9:02 [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR Aldy Hernandez
@ 2022-11-09 14:58 ` Jakub Jelinek
  2022-11-09 15:43   ` Aldy Hernandez
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2022-11-09 14:58 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR.
  2022-11-09 14:58 ` Jakub Jelinek
@ 2022-11-09 15:43   ` Aldy Hernandez
  2022-11-09 15:53     ` Jakub Jelinek
  2022-11-11 11:50     ` [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR Jakub Jelinek
  0 siblings, 2 replies; 7+ messages in thread
From: Aldy Hernandez @ 2022-11-09 15:43 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC patches, Andrew MacLeod, Xi Ruoyao

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

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

[-- Attachment #2: 0001-Revert-op-12-_range-operators-for-PLUS_EXPR-and-MINU.patch --]
[-- Type: text/x-patch, Size: 2781 bytes --]

From f8ac2b15d1ce8cbb2f4a4ba89afbd87a8aa5c4e6 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Wed, 9 Nov 2022 16:35:40 +0100
Subject: [PATCH] Revert op[12]_range operators for PLUS_EXPR and MINUS_EXPR.

Revert the patch below until issues are resolved:

	commit 4287e8168f89e90b3dff3a50f3ada40be53e0e01
	Author: Aldy Hernandez <aldyh@redhat.com>
	Date:   Wed Nov 9 01:00:57 2022 +0100

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

	    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 (class foperator_plus): Remove op[12]_range.
	(class foperator_minus): Same.
---
 gcc/range-op-float.cc | 45 -------------------------------------------
 1 file changed, 45 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index cc806438a19..380142b4c14 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -1863,29 +1863,6 @@ 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,
@@ -1910,28 +1887,6 @@ private:
 
 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,
-- 
2.38.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR.
  2022-11-09 15:43   ` Aldy Hernandez
@ 2022-11-09 15:53     ` Jakub Jelinek
  2022-11-11 11:50     ` [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR Jakub Jelinek
  1 sibling, 0 replies; 7+ messages in thread
From: Jakub Jelinek @ 2022-11-09 15:53 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod, Xi Ruoyao

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR
  2022-11-09 15:43   ` Aldy Hernandez
  2022-11-09 15:53     ` Jakub Jelinek
@ 2022-11-11 11:50     ` Jakub Jelinek
  2022-11-11 14:22       ` Aldy Hernandez
  1 sibling, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2022-11-11 11:50 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: gcc-patches

On Wed, Nov 09, 2022 at 04:43:56PM +0100, Aldy Hernandez wrote:
> 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.

Here is my (so far only on the testcase tested) patch which reinstalls
your change, add the fixups I've talked about and also hooks up
reverse operators for MULT_EXPR/RDIV_EXPR.

2022-11-11  Aldy Hernandez  <aldyh@redhat.com>
	    Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (float_binary_op_range_finish): New function.
	(foperator_plus::op1_range): New.
        (foperator_plus::op2_range): New.
        (foperator_minus::op1_range): New.
        (foperator_minus::op2_range): New.
	(foperator_mult::op1_range): New.
        (foperator_mult::op2_range): New.
        (foperator_div::op1_range): New.
        (foperator_div::op2_range): New.

	* gcc.c-torture/execute/ieee/inf-4.c: New test.

--- gcc/range-op-float.cc.jj	2022-11-11 10:55:57.602617289 +0100
+++ gcc/range-op-float.cc	2022-11-11 12:32:19.378633983 +0100
@@ -1861,8 +1861,64 @@ foperator_unordered_equal::op1_range (fr
   return true;
 }
 
+// Final tweaks for float binary op op1_range/op2_range.
+
+static bool
+float_binary_op_range_finish (bool ret, frange &r, tree type,
+			      const frange &lhs)
+{
+  if (!ret)
+    return ret;
+
+  // If we get a known NAN from reverse op, it means either that
+  // the other operand was known NAN (in that case we know nothing),
+  // or the reverse operation introduced a known NAN.
+  // Say for lhs = op1 * op2 if lhs is [-0, +0] and op2 is too,
+  // 0 / 0 is known NAN.  Just punt in that case.
+  // Or if lhs is a known NAN, we also don't know anything.
+  if (r.known_isnan () || lhs.known_isnan ())
+    {
+      r.set_varying (type);
+      return false;
+    }
+
+  // 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 ();
+  // If lhs is a maybe or known NAN, the operand could be
+  // NAN.
+  else
+    r.update_nan ();
+  return true;
+}
+
 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 float_binary_op_range_finish (minus.fold_range (r, type, lhs, op2),
+					 r, type, lhs);
+  }
+  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,
@@ -1888,6 +1944,31 @@ class foperator_plus : public range_oper
 
 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 float_binary_op_range_finish (fop_plus.fold_range (r, type, lhs,
+							      op2),
+					 r, type, lhs);
+  }
+  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 float_binary_op_range_finish (fold_range (r, type, op1, lhs),
+					 r, type, lhs);
+  }
+private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
 		tree type,
 		const REAL_VALUE_TYPE &lh_lb,
@@ -2031,6 +2112,30 @@ protected:
 
 class foperator_mult : public foperator_mult_div_base
 {
+  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 rdiv (RDIV_EXPR, type);
+    if (!rdiv)
+      return false;
+    return float_binary_op_range_finish (rdiv.fold_range (r, type, lhs, op2),
+					 r, type, lhs);
+  }
+  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,
@@ -2138,6 +2243,31 @@ class foperator_mult : public foperator_
 
 class foperator_div : public foperator_mult_div_base
 {
+  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 float_binary_op_range_finish (fop_mult.fold_range (r, type, lhs,
+							      op2),
+					 r, type, lhs);
+  }
+  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 float_binary_op_range_finish (fold_range (r, type, op1, lhs),
+					 r, type, lhs);
+  }
+private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
 		tree type,
 		const REAL_VALUE_TYPE &lh_lb,
--- gcc/testsuite/gcc.c-torture/execute/ieee/inf-4.c.jj	2022-11-11 12:44:57.615274471 +0100
+++ gcc/testsuite/gcc.c-torture/execute/ieee/inf-4.c	2022-11-11 12:44:13.351879222 +0100
@@ -0,0 +1,26 @@
+__attribute__((noipa)) int
+foo (double a, double b)
+{
+  double c = a - b;
+  if (!__builtin_isfinite (c))
+    {
+      if (__builtin_isnan (c))
+	{
+	  if (!__builtin_isnan (a) && !__builtin_isnan (b))
+	    return 1;
+	}
+      else if (__builtin_isfinite (a) && __builtin_isfinite (b))
+	return 2;
+    }
+  else if (c == 0 && a != b)
+    return 3;
+  return 4;
+}
+
+int
+main ()
+{
+  double a = __builtin_inf ();
+  if (foo (a, a) != 1)
+    __builtin_abort ();
+}


	Jakub


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR
  2022-11-11 11:50     ` [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR Jakub Jelinek
@ 2022-11-11 14:22       ` Aldy Hernandez
  2022-11-11 14:58         ` Andrew MacLeod
  0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2022-11-11 14:22 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, amacleod



On 11/11/22 12:50, Jakub Jelinek wrote:
> On Wed, Nov 09, 2022 at 04:43:56PM +0100, Aldy Hernandez wrote:
>> 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.
> 
> Here is my (so far only on the testcase tested) patch which reinstalls
> your change, add the fixups I've talked about and also hooks up
> reverse operators for MULT_EXPR/RDIV_EXPR.

OMG, you're a rockstar (or salsa or bachata star if that's your thing)! 
:-P).

Thank you so much.  I was just looking at that now.

> 
> 2022-11-11  Aldy Hernandez  <aldyh@redhat.com>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (float_binary_op_range_finish): New function.
> 	(foperator_plus::op1_range): New.
>          (foperator_plus::op2_range): New.
>          (foperator_minus::op1_range): New.
>          (foperator_minus::op2_range): New.
> 	(foperator_mult::op1_range): New.
>          (foperator_mult::op2_range): New.
>          (foperator_div::op1_range): New.
>          (foperator_div::op2_range): New.
> 
> 	* gcc.c-torture/execute/ieee/inf-4.c: New test.
> 
> --- gcc/range-op-float.cc.jj	2022-11-11 10:55:57.602617289 +0100
> +++ gcc/range-op-float.cc	2022-11-11 12:32:19.378633983 +0100
> @@ -1861,8 +1861,64 @@ foperator_unordered_equal::op1_range (fr
>     return true;
>   }
>   
> +// Final tweaks for float binary op op1_range/op2_range.
> +
> +static bool
> +float_binary_op_range_finish (bool ret, frange &r, tree type,
> +			      const frange &lhs)
> +{

Can you document the return value, even if it's just "the same as 
op1/2_range" ;-).

> +  if (!ret)
> +    return ret;
> +
> +  // If we get a known NAN from reverse op, it means either that
> +  // the other operand was known NAN (in that case we know nothing),
> +  // or the reverse operation introduced a known NAN.
> +  // Say for lhs = op1 * op2 if lhs is [-0, +0] and op2 is too,
> +  // 0 / 0 is known NAN.  Just punt in that case.
> +  // Or if lhs is a known NAN, we also don't know anything.
> +  if (r.known_isnan () || lhs.known_isnan ())
> +    {
> +      r.set_varying (type);
> +      return false;
> +    }

A return of false means the operation is not handled, similar to what 
the default operators defined at the top of range-op*.cc do.  The caller 
(gori?) is free to disregard the range altogether.  In practice this 
means VARYING, so you're getting the same behavior.  But you should 
probably return TRUE, which is what we do in other operators. 
Technically you could also not set "r" and just return false.

Otherwise LGTM.

I'll look at your other patches next.
Aldy

> +
> +  // 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 ();
> +  // If lhs is a maybe or known NAN, the operand could be
> +  // NAN.
> +  else
> +    r.update_nan ();
> +  return true;
> +}
> +
>   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 float_binary_op_range_finish (minus.fold_range (r, type, lhs, op2),
> +					 r, type, lhs);
> +  } > +  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,
> @@ -1888,6 +1944,31 @@ class foperator_plus : public range_oper
>   
>   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 float_binary_op_range_finish (fop_plus.fold_range (r, type, lhs,
> +							      op2),
> +					 r, type, lhs);
> +  }
> +  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 float_binary_op_range_finish (fold_range (r, type, op1, lhs),
> +					 r, type, lhs);
> +  }
> +private:
>     void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
>   		tree type,
>   		const REAL_VALUE_TYPE &lh_lb,
> @@ -2031,6 +2112,30 @@ protected:
>   
>   class foperator_mult : public foperator_mult_div_base
>   {
> +  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 rdiv (RDIV_EXPR, type);
> +    if (!rdiv)
> +      return false;
> +    return float_binary_op_range_finish (rdiv.fold_range (r, type, lhs, op2),
> +					 r, type, lhs);
> +  }
> +  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,
> @@ -2138,6 +2243,31 @@ class foperator_mult : public foperator_
>   
>   class foperator_div : public foperator_mult_div_base
>   {
> +  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 float_binary_op_range_finish (fop_mult.fold_range (r, type, lhs,
> +							      op2),
> +					 r, type, lhs);
> +  }
> +  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 float_binary_op_range_finish (fold_range (r, type, op1, lhs),
> +					 r, type, lhs);
> +  }
> +private:
>     void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
>   		tree type,
>   		const REAL_VALUE_TYPE &lh_lb,
> --- gcc/testsuite/gcc.c-torture/execute/ieee/inf-4.c.jj	2022-11-11 12:44:57.615274471 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/ieee/inf-4.c	2022-11-11 12:44:13.351879222 +0100
> @@ -0,0 +1,26 @@
> +__attribute__((noipa)) int
> +foo (double a, double b)
> +{
> +  double c = a - b;
> +  if (!__builtin_isfinite (c))
> +    {
> +      if (__builtin_isnan (c))
> +	{
> +	  if (!__builtin_isnan (a) && !__builtin_isnan (b))
> +	    return 1;
> +	}
> +      else if (__builtin_isfinite (a) && __builtin_isfinite (b))
> +	return 2;
> +    }
> +  else if (c == 0 && a != b)
> +    return 3;
> +  return 4;
> +}
> +
> +int
> +main ()
> +{
> +  double a = __builtin_inf ();
> +  if (foo (a, a) != 1)
> +    __builtin_abort ();
> +}
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR
  2022-11-11 14:22       ` Aldy Hernandez
@ 2022-11-11 14:58         ` Andrew MacLeod
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew MacLeod @ 2022-11-11 14:58 UTC (permalink / raw)
  To: Aldy Hernandez, Jakub Jelinek; +Cc: gcc-patches


On 11/11/22 09:22, Aldy Hernandez wrote:
>
>
>
> A return of false means the operation is not handled, similar to what 
> the default operators defined at the top of range-op*.cc do. The 
> caller (gori?) is free to disregard the range altogether.  In practice 
> this means VARYING, so you're getting the same behavior. But you 
> should probably return TRUE,


When false is returned, the range i suppose to be ignored as it is not 
guaranteed to be set.  It means, "I cant tell you anything additional to 
what is already known".  (which is similar to returning VARYING..)

Andrew


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-11-11 14:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-09  9:02 [COMMITTED] Implement op[12]_range operators for PLUS_EXPR and MINUS_EXPR Aldy Hernandez
2022-11-09 14:58 ` Jakub Jelinek
2022-11-09 15:43   ` Aldy Hernandez
2022-11-09 15:53     ` Jakub Jelinek
2022-11-11 11:50     ` [PATCH] range-op: Implement op[12]_range operators for {PLUS,MINUS,MULT,RDIV}_EXPR Jakub Jelinek
2022-11-11 14:22       ` Aldy Hernandez
2022-11-11 14:58         ` Andrew MacLeod

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).