diff mbox series

[3/5] Avoid upper/lower_bound (1) on VR_ANTI_RANGE

Message ID 20230228134733.417D113440@imap2.suse-dmz.suse.de
State Accepted
Headers
Series Fix irange::legacy_upper_bound |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Richard Biener Feb. 28, 2023, 1:47 p.m. UTC 
  simplify_using_ranges::two_valued_val_range_p has odd code to
verify that an anti-range is two-valued which relies on
num_pairs () returning two for anti-ranges despite there's only
one pair and relying on lower/upper_bound treating that argument
specially.  I cannot convince myself that's even correct.
The following avoids this by using a temporary int_range<2> to
allow anti-ranges to be represented as union of two ranges.

	* vr-values.cc (simplify_using_ranges::two_valued_val_range_p):
	Canonicalize legacy range to int_range<2> before checking
	for two valued-ness.
---
 gcc/vr-values.cc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Aldy Hernandez March 1, 2023, 10:34 a.m. UTC | #1 
On 2/28/23 14:47, Richard Biener via Gcc-patches wrote:
> simplify_using_ranges::two_valued_val_range_p has odd code to
> verify that an anti-range is two-valued which relies on
> num_pairs () returning two for anti-ranges despite there's only
> one pair and relying on lower/upper_bound treating that argument
> specially.  I cannot convince myself that's even correct.
> The following avoids this by using a temporary int_range<2> to
> allow anti-ranges to be represented as union of two ranges.
> 
> 	* vr-values.cc (simplify_using_ranges::two_valued_val_range_p):
> 	Canonicalize legacy range to int_range<2> before checking
> 	for two valued-ness.
> ---
>   gcc/vr-values.cc | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> index 365f4976a39..c188fff1f8c 100644
> --- a/gcc/vr-values.cc
> +++ b/gcc/vr-values.cc
> @@ -2189,11 +2189,12 @@ bool
>   simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b,
>   					       gimple *s)
>   {
> -  value_range vr = *query->get_value_range (var, s);
> -  vr.normalize_symbolics ();
> -  if (vr.varying_p () || vr.undefined_p ())
> +  value_range r = *query->get_value_range (var, s);
> +  r.normalize_symbolics ();
> +  if (r.varying_p () || r.undefined_p ())
>       return false;
>   
> +  int_range<2> vr (r);
>     if ((vr.num_pairs () == 1 && vr.upper_bound () - vr.lower_bound () == 1)
>         || (vr.num_pairs () == 2
>   	  && vr.lower_bound (0) == vr.upper_bound (0)

This is all fixed in the new code, but no harm in fixing it now.  I'll 
just merge my work with yours.

OK, thanks.
diff mbox series

Patch

diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
index 365f4976a39..c188fff1f8c 100644
--- a/gcc/vr-values.cc
+++ b/gcc/vr-values.cc
@@ -2189,11 +2189,12 @@  bool
 simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b,
 					       gimple *s)
 {
-  value_range vr = *query->get_value_range (var, s);
-  vr.normalize_symbolics ();
-  if (vr.varying_p () || vr.undefined_p ())
+  value_range r = *query->get_value_range (var, s);
+  r.normalize_symbolics ();
+  if (r.varying_p () || r.undefined_p ())
     return false;
 
+  int_range<2> vr (r);
   if ((vr.num_pairs () == 1 && vr.upper_bound () - vr.lower_bound () == 1)
       || (vr.num_pairs () == 2
 	  && vr.lower_bound (0) == vr.upper_bound (0)