[PATCH 4/5] Sanitize irange::num_pairs

[PATCH 4/5] Sanitize irange::num_pairs

[Richard Biener]

irange::num_pairs has odd behavior for VR_ANTI_RANGE where it
claims there are two pairs when there's actually only one.  The
following is now able to get rid of this, also fixing
irange::legacy_upper_bound which special-cased ~[-INF, up]
to return +INF instead of properly doing that when up is not +INF.

	* value-range.h (irange::num_pairs): Always return
	m_num_ranges.
	* value-range.cc (irange::legacy_lower_bound): Remove
	pair == 1 case.
	(irange::legacy_upper_bound): Likewise.  Properly
	special-case ~[low, +INF].
---
 gcc/value-range.cc | 6 ++++--
 gcc/value-range.h  | 5 +----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index a535337c47a..143af5601c7 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -1208,7 +1208,8 @@ irange::legacy_lower_bound (unsigned pair) const
   if (m_kind == VR_ANTI_RANGE)
     {
       tree typ = type (), t;
-      if (pair == 1 || vrp_val_is_min (min ()))
+      gcc_checking_assert (pair == 0);
+      if (vrp_val_is_min (min ()))
 	t = wide_int_to_tree (typ, wi::to_wide (max ()) + 1);
       else
 	t = vrp_val_min (typ);
@@ -1235,7 +1236,8 @@ irange::legacy_upper_bound (unsigned pair) const
   if (m_kind == VR_ANTI_RANGE)
     {
       tree typ = type (), t;
-      if (pair == 1 || vrp_val_is_min (min ()))
+      gcc_checking_assert (pair == 0);
+      if (!vrp_val_is_max (max ()))
 	t = vrp_val_max (typ);
       else
 	t = wide_int_to_tree (typ, wi::to_wide (min ()) - 1);
diff --git a/gcc/value-range.h b/gcc/value-range.h
index f4ac73b499f..cfb51bad915 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -614,10 +614,7 @@ vrange::kind () const
 inline unsigned
 irange::num_pairs () const
 {
-  if (m_kind == VR_ANTI_RANGE)
-    return constant_p () ? 2 : 1;
-  else
-    return m_num_ranges;
+  return m_num_ranges;
 }
 
 inline tree
-- 
2.35.3

Re: [PATCH 4/5] Sanitize irange::num_pairs

[Aldy Hernandez]

I would prefer not touching this as it was intended, and about to be 
removed.  However, if we have actual regressions or missed optimizations 
because of the current behavior I could be convinced otherwise.

Aldy

On 2/28/23 14:47, Richard Biener via Gcc-patches wrote:
> irange::num_pairs has odd behavior for VR_ANTI_RANGE where it
> claims there are two pairs when there's actually only one.  The
> following is now able to get rid of this, also fixing
> irange::legacy_upper_bound which special-cased ~[-INF, up]
> to return +INF instead of properly doing that when up is not +INF.
> 
> 	* value-range.h (irange::num_pairs): Always return
> 	m_num_ranges.
> 	* value-range.cc (irange::legacy_lower_bound): Remove
> 	pair == 1 case.
> 	(irange::legacy_upper_bound): Likewise.  Properly
> 	special-case ~[low, +INF].
> ---
>   gcc/value-range.cc | 6 ++++--
>   gcc/value-range.h  | 5 +----
>   2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index a535337c47a..143af5601c7 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -1208,7 +1208,8 @@ irange::legacy_lower_bound (unsigned pair) const
>     if (m_kind == VR_ANTI_RANGE)
>       {
>         tree typ = type (), t;
> -      if (pair == 1 || vrp_val_is_min (min ()))
> +      gcc_checking_assert (pair == 0);
> +      if (vrp_val_is_min (min ()))
>   	t = wide_int_to_tree (typ, wi::to_wide (max ()) + 1);
>         else
>   	t = vrp_val_min (typ);
> @@ -1235,7 +1236,8 @@ irange::legacy_upper_bound (unsigned pair) const
>     if (m_kind == VR_ANTI_RANGE)
>       {
>         tree typ = type (), t;
> -      if (pair == 1 || vrp_val_is_min (min ()))
> +      gcc_checking_assert (pair == 0);
> +      if (!vrp_val_is_max (max ()))
>   	t = vrp_val_max (typ);
>         else
>   	t = wide_int_to_tree (typ, wi::to_wide (min ()) - 1);
> diff --git a/gcc/value-range.h b/gcc/value-range.h
> index f4ac73b499f..cfb51bad915 100644
> --- a/gcc/value-range.h
> +++ b/gcc/value-range.h
> @@ -614,10 +614,7 @@ vrange::kind () const
>   inline unsigned
>   irange::num_pairs () const
>   {
> -  if (m_kind == VR_ANTI_RANGE)
> -    return constant_p () ? 2 : 1;
> -  else
> -    return m_num_ranges;
> +  return m_num_ranges;
>   }
>   
>   inline tree