Re: [range-ops] patch 02/04: enforce canonicalization in value_range

[Jeff Law <law at redhat dot com>]

On 7/1/19 3:02 AM, Aldy Hernandez wrote:
> As discussed prior.  This enforces canonicalization at creation time,
> which makes things a lot more consistent throughout.
> 
> Since now [MIN, MAX] will be canonicalized into VR_VARYING, we can no
> longer depend on normalizing VARYING's into [MIN, MAX] for more general
> handling.  I've adjusted the few places where we were doing this so that
> they work correctly.
> 
> As a bonus, now that we're canonicalized, I've tweaked singleton to work
> with anti-ranges.
> 
> Note: There is a small note in ranges_from_anti_range, which I will
> start making heavier use of in subsequent patches:
> 
> +  /* ?? This function is called multiple times from num_pairs,
> +     lower_bound, and upper_bound.  We should probably memoize this, or
> +     rewrite the callers in such a way that we're not re-calculating
> +     this constantly.  */
> 
> I don't think this is needed now, but just a note of things to come.  It
> may not be an issue, but just something to keep in mind.
> 
> Tested on x86-64 Linux with --enable-languages=all.
> 
> Aldy
> 
> range-ops-canon.patch
> 
> commit d220a3eeb77615b39260e532e34815a3810e8348
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Fri Jun 28 11:15:03 2019 +0200
> 
>     Enforce canonicalization in value_range.
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index a5247735694..866200d9594 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,20 @@
> +2019-07-01  Aldy Hernandez  <aldyh@redhat.com>
> +
> +	* tree-vrp.c (value_range_base::set_and_canonicalize): Rename to
> +	set() and add normalization of [MIN, MAX] into varying.
> +	(value_range_base::singleton_p): Make work with anti ranges.
> +	(ranges_from_anti_range): Handle pointers.
> +	(extract_range_into_wide_ints): Call normalize_symbolics.
> +	(extract_range_from_binary_expr): Do not build a [MIN,MAX] range
> +	because it will be canonicalized into varying.
> +	Call set() instead of set_and_canonicalize().
> +	(extract_range_from_unary_expr): Call set() instead of
> +	set_and_canonicalize().
> +	(intersect_helper): Do not call set_and_canonicalize.
> +	* tree-vrp.h (value_range_base): Remove set_and_canonicalize.
> +	(value_range): Same.
> +	* vr-values.c (extract_range_from_var_from_comparison_expr): Same.
I don't see anything inherently concerning here.  I do wonder if there's
any value in having a debugging function in the class that would iterate
over the ranges and check them for proper canonicalization, verify that
VR_{VARYING,UNDEFINED} objects do not have equivalences, etc.  Thoughts?

Jeff