public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] range-op: Handle op?.undefined_p () in op[12]_range of comparisons [PR108647]
@ 2023-02-03 18:59 Jakub Jelinek
  2023-02-03 20:29 ` Aldy Hernandez
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-02-03 18:59 UTC (permalink / raw)
  To: Aldy Hernandez, Andrew MacLeod; +Cc: gcc-patches

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

Hi!

As mentioned in the PR, we ICE because lhs is singleton [0, 0]
or [1, 1] but op2 (or in other cases op1) is undefined and op?.*_bound ()
ICEs on those because there are no pairs for UNDEFINED.

The following patch makes us set r to varying or return false in those
cases.

Included is a version of the patch I've bootstrapped/regtested successfully
on x86_64-linux and i686-linux, attached is a slight modification more
consistent with the range-op-float.cc patch.

Ok for trunk (and which one)?

2023-02-03  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/108647
	* range-op.cc (operator_equal::op1_range,
	operator_not_equal::op1_range): Don't test op2 bound
	equality if op2.undefined_p (), instead set_varying.
	(operator_lt::op1_range, operator_le::op1_range,
	operator_gt::op1_range, operator_ge::op1_range): Return false if
	op2.undefined_p ().
	(operator_lt::op2_range, operator_le::op2_range,
	operator_gt::op2_range, operator_ge::op2_range): Return false if
	op1.undefined_p ().

	* g++.dg/torture/pr108647.C: New test.

--- gcc/range-op.cc.jj	2023-02-03 10:51:40.699003658 +0100
+++ gcc/range-op.cc	2023-02-03 17:26:02.204429931 +0100
@@ -642,7 +642,8 @@ operator_equal::op1_range (irange &r, tr
     case BRS_FALSE:
       // If the result is false, the only time we know anything is
       // if OP2 is a constant.
-      if (wi::eq_p (op2.lower_bound(), op2.upper_bound()))
+      if (!op2.undefined_p ()
+	  && wi::eq_p (op2.lower_bound(), op2.upper_bound()))
 	{
 	  r = op2;
 	  r.invert ();
@@ -755,7 +756,8 @@ operator_not_equal::op1_range (irange &r
     case BRS_TRUE:
       // If the result is true, the only time we know anything is if
       // OP2 is a constant.
-      if (wi::eq_p (op2.lower_bound(), op2.upper_bound()))
+      if (!op2.undefined_p ()
+	  && wi::eq_p (op2.lower_bound(), op2.upper_bound()))
 	{
 	  r = op2;
 	  r.invert ();
@@ -920,6 +922,9 @@ operator_lt::op1_range (irange &r, tree
 			const irange &op2,
 			relation_trio) const
 {
+  if (op2.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -942,6 +947,9 @@ operator_lt::op2_range (irange &r, tree
 			const irange &op1,
 			relation_trio) const
 {
+  if (op1.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -1031,6 +1039,9 @@ operator_le::op1_range (irange &r, tree
 			const irange &op2,
 			relation_trio) const
 {
+  if (op2.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -1053,6 +1064,9 @@ operator_le::op2_range (irange &r, tree
 			const irange &op1,
 			relation_trio) const
 {
+  if (op1.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -1141,6 +1155,9 @@ operator_gt::op1_range (irange &r, tree
 			const irange &lhs, const irange &op2,
 			relation_trio) const
 {
+  if (op2.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -1163,6 +1180,9 @@ operator_gt::op2_range (irange &r, tree
 			const irange &op1,
 			relation_trio) const
 {
+  if (op1.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -1252,6 +1272,9 @@ operator_ge::op1_range (irange &r, tree
 			const irange &op2,
 			relation_trio) const
 {
+  if (op2.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
@@ -1274,6 +1297,9 @@ operator_ge::op2_range (irange &r, tree
 			const irange &op1,
 			relation_trio) const
 {
+  if (op1.undefined_p ())
+    return false;
+
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
--- gcc/testsuite/g++.dg/torture/pr108647.C.jj	2023-02-03 16:36:18.347255058 +0100
+++ gcc/testsuite/g++.dg/torture/pr108647.C	2023-02-03 16:32:16.338811259 +0100
@@ -0,0 +1,25 @@
+// PR tree-optimization/108647
+// { dg-do compile }
+
+bool a;
+int b, c;
+
+inline const bool &
+foo (bool &e, const bool &f)
+{
+  return f < e ? f : e;
+}
+
+void
+bar (signed char e, bool *f, bool *h, bool *g)
+{
+  for (;;)
+    if (g)
+      for (signed char j = 0; j < 6;
+	   j += ((f[0] & c ? g[0] : int(0 >= e))
+		 ? 0 : foo (g[0], g[0] > h[0]) + 1))
+	{
+	  a = 0;
+	  b = 0;
+	}
+}

	Jakub

[-- Attachment #2: R782a --]
[-- Type: text/plain, Size: 4947 bytes --]

2023-02-03  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/108647
	* range-op.cc (operator_equal::op1_range,
	operator_not_equal::op1_range): Don't test op2 bound
	equality if op2.undefined_p (), instead set_varying.
	(operator_lt::op1_range, operator_le::op1_range,
	operator_gt::op1_range, operator_ge::op1_range): Return false if
	op2.undefined_p ().
	(operator_lt::op2_range, operator_le::op2_range,
	operator_gt::op2_range, operator_ge::op2_range): Return false if
	op1.undefined_p ().

	* g++.dg/torture/pr108647.C: New test.

--- gcc/range-op.cc.jj	2023-02-03 10:51:40.699003658 +0100
+++ gcc/range-op.cc	2023-02-03 19:43:28.082326069 +0100
@@ -642,7 +642,8 @@ operator_equal::op1_range (irange &r, tr
     case BRS_FALSE:
       // If the result is false, the only time we know anything is
       // if OP2 is a constant.
-      if (wi::eq_p (op2.lower_bound(), op2.upper_bound()))
+      if (!op2.undefined_p ()
+	  && wi::eq_p (op2.lower_bound(), op2.upper_bound()))
 	{
 	  r = op2;
 	  r.invert ();
@@ -755,7 +756,8 @@ operator_not_equal::op1_range (irange &r
     case BRS_TRUE:
       // If the result is true, the only time we know anything is if
       // OP2 is a constant.
-      if (wi::eq_p (op2.lower_bound(), op2.upper_bound()))
+      if (!op2.undefined_p ()
+	  && wi::eq_p (op2.lower_bound(), op2.upper_bound()))
 	{
 	  r = op2;
 	  r.invert ();
@@ -923,10 +925,14 @@ operator_lt::op1_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op2.undefined_p ())
+	return false;
       build_lt (r, type, op2.upper_bound ());
       break;
 
     case BRS_FALSE:
+      if (op2.undefined_p ())
+	return false;
       build_ge (r, type, op2.lower_bound ());
       break;
 
@@ -945,10 +951,14 @@ operator_lt::op2_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op1.undefined_p ())
+	return false;
       build_gt (r, type, op1.lower_bound ());
       break;
 
     case BRS_FALSE:
+      if (op1.undefined_p ())
+	return false;
       build_le (r, type, op1.upper_bound ());
       break;
 
@@ -1034,10 +1044,14 @@ operator_le::op1_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op2.undefined_p ())
+	return false;
       build_le (r, type, op2.upper_bound ());
       break;
 
     case BRS_FALSE:
+      if (op2.undefined_p ())
+	return false;
       build_gt (r, type, op2.lower_bound ());
       break;
 
@@ -1056,10 +1070,14 @@ operator_le::op2_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op1.undefined_p ())
+	return false;
       build_ge (r, type, op1.lower_bound ());
       break;
 
     case BRS_FALSE:
+      if (op1.undefined_p ())
+	return false;
       build_lt (r, type, op1.upper_bound ());
       break;
 
@@ -1144,10 +1162,14 @@ operator_gt::op1_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op2.undefined_p ())
+	return false;
       build_gt (r, type, op2.lower_bound ());
       break;
 
     case BRS_FALSE:
+      if (op2.undefined_p ())
+	return false;
       build_le (r, type, op2.upper_bound ());
       break;
 
@@ -1166,10 +1188,14 @@ operator_gt::op2_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op1.undefined_p ())
+	return false;
       build_lt (r, type, op1.upper_bound ());
       break;
 
     case BRS_FALSE:
+      if (op1.undefined_p ())
+	return false;
       build_ge (r, type, op1.lower_bound ());
       break;
 
@@ -1255,10 +1281,14 @@ operator_ge::op1_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op2.undefined_p ())
+	return false;
       build_ge (r, type, op2.lower_bound ());
       break;
 
     case BRS_FALSE:
+      if (op2.undefined_p ())
+	return false;
       build_lt (r, type, op2.upper_bound ());
       break;
 
@@ -1277,10 +1307,14 @@ operator_ge::op2_range (irange &r, tree
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
+      if (op1.undefined_p ())
+	return false;
       build_le (r, type, op1.upper_bound ());
       break;
 
     case BRS_FALSE:
+      if (op1.undefined_p ())
+	return false;
       build_gt (r, type, op1.lower_bound ());
       break;
 
--- gcc/testsuite/g++.dg/torture/pr108647.C.jj	2023-02-03 16:36:18.347255058 +0100
+++ gcc/testsuite/g++.dg/torture/pr108647.C	2023-02-03 16:32:16.338811259 +0100
@@ -0,0 +1,25 @@
+// PR tree-optimization/108647
+// { dg-do compile }
+
+bool a;
+int b, c;
+
+inline const bool &
+foo (bool &e, const bool &f)
+{
+  return f < e ? f : e;
+}
+
+void
+bar (signed char e, bool *f, bool *h, bool *g)
+{
+  for (;;)
+    if (g)
+      for (signed char j = 0; j < 6;
+	   j += ((f[0] & c ? g[0] : int(0 >= e))
+		 ? 0 : foo (g[0], g[0] > h[0]) + 1))
+	{
+	  a = 0;
+	  b = 0;
+	}
+}

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

* Re: [PATCH] range-op: Handle op?.undefined_p () in op[12]_range of comparisons [PR108647]
  2023-02-03 18:59 [PATCH] range-op: Handle op?.undefined_p () in op[12]_range of comparisons [PR108647] Jakub Jelinek
@ 2023-02-03 20:29 ` Aldy Hernandez
  0 siblings, 0 replies; 2+ messages in thread
From: Aldy Hernandez @ 2023-02-03 20:29 UTC (permalink / raw)
  To: Jakub Jelinek, Andrew MacLeod; +Cc: gcc-patches

LGTM

On 2/3/23 19:59, Jakub Jelinek wrote:
> Hi!
> 
> As mentioned in the PR, we ICE because lhs is singleton [0, 0]
> or [1, 1] but op2 (or in other cases op1) is undefined and op?.*_bound ()
> ICEs on those because there are no pairs for UNDEFINED.
> 
> The following patch makes us set r to varying or return false in those
> cases.
> 
> Included is a version of the patch I've bootstrapped/regtested successfully
> on x86_64-linux and i686-linux, attached is a slight modification more
> consistent with the range-op-float.cc patch.
> 
> Ok for trunk (and which one)?
> 
> 2023-02-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/108647
> 	* range-op.cc (operator_equal::op1_range,
> 	operator_not_equal::op1_range): Don't test op2 bound
> 	equality if op2.undefined_p (), instead set_varying.
> 	(operator_lt::op1_range, operator_le::op1_range,
> 	operator_gt::op1_range, operator_ge::op1_range): Return false if
> 	op2.undefined_p ().
> 	(operator_lt::op2_range, operator_le::op2_range,
> 	operator_gt::op2_range, operator_ge::op2_range): Return false if
> 	op1.undefined_p ().
> 
> 	* g++.dg/torture/pr108647.C: New test.
> 
> --- gcc/range-op.cc.jj	2023-02-03 10:51:40.699003658 +0100
> +++ gcc/range-op.cc	2023-02-03 17:26:02.204429931 +0100
> @@ -642,7 +642,8 @@ operator_equal::op1_range (irange &r, tr
>       case BRS_FALSE:
>         // If the result is false, the only time we know anything is
>         // if OP2 is a constant.
> -      if (wi::eq_p (op2.lower_bound(), op2.upper_bound()))
> +      if (!op2.undefined_p ()
> +	  && wi::eq_p (op2.lower_bound(), op2.upper_bound()))
>   	{
>   	  r = op2;
>   	  r.invert ();
> @@ -755,7 +756,8 @@ operator_not_equal::op1_range (irange &r
>       case BRS_TRUE:
>         // If the result is true, the only time we know anything is if
>         // OP2 is a constant.
> -      if (wi::eq_p (op2.lower_bound(), op2.upper_bound()))
> +      if (!op2.undefined_p ()
> +	  && wi::eq_p (op2.lower_bound(), op2.upper_bound()))
>   	{
>   	  r = op2;
>   	  r.invert ();
> @@ -920,6 +922,9 @@ operator_lt::op1_range (irange &r, tree
>   			const irange &op2,
>   			relation_trio) const
>   {
> +  if (op2.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -942,6 +947,9 @@ operator_lt::op2_range (irange &r, tree
>   			const irange &op1,
>   			relation_trio) const
>   {
> +  if (op1.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -1031,6 +1039,9 @@ operator_le::op1_range (irange &r, tree
>   			const irange &op2,
>   			relation_trio) const
>   {
> +  if (op2.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -1053,6 +1064,9 @@ operator_le::op2_range (irange &r, tree
>   			const irange &op1,
>   			relation_trio) const
>   {
> +  if (op1.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -1141,6 +1155,9 @@ operator_gt::op1_range (irange &r, tree
>   			const irange &lhs, const irange &op2,
>   			relation_trio) const
>   {
> +  if (op2.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -1163,6 +1180,9 @@ operator_gt::op2_range (irange &r, tree
>   			const irange &op1,
>   			relation_trio) const
>   {
> +  if (op1.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -1252,6 +1272,9 @@ operator_ge::op1_range (irange &r, tree
>   			const irange &op2,
>   			relation_trio) const
>   {
> +  if (op2.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> @@ -1274,6 +1297,9 @@ operator_ge::op2_range (irange &r, tree
>   			const irange &op1,
>   			relation_trio) const
>   {
> +  if (op1.undefined_p ())
> +    return false;
> +
>     switch (get_bool_state (r, lhs, type))
>       {
>       case BRS_TRUE:
> --- gcc/testsuite/g++.dg/torture/pr108647.C.jj	2023-02-03 16:36:18.347255058 +0100
> +++ gcc/testsuite/g++.dg/torture/pr108647.C	2023-02-03 16:32:16.338811259 +0100
> @@ -0,0 +1,25 @@
> +// PR tree-optimization/108647
> +// { dg-do compile }
> +
> +bool a;
> +int b, c;
> +
> +inline const bool &
> +foo (bool &e, const bool &f)
> +{
> +  return f < e ? f : e;
> +}
> +
> +void
> +bar (signed char e, bool *f, bool *h, bool *g)
> +{
> +  for (;;)
> +    if (g)
> +      for (signed char j = 0; j < 6;
> +	   j += ((f[0] & c ? g[0] : int(0 >= e))
> +		 ? 0 : foo (g[0], g[0] > h[0]) + 1))
> +	{
> +	  a = 0;
> +	  b = 0;
> +	}
> +}
> 
> 	Jakub


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

end of thread, other threads:[~2023-02-03 20:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03 18:59 [PATCH] range-op: Handle op?.undefined_p () in op[12]_range of comparisons [PR108647] Jakub Jelinek
2023-02-03 20:29 ` Aldy Hernandez

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).