enforce canonicalization of value_range's

[Jeff Law]

On 8/12/19 12:48 PM, Aldy Hernandez wrote:
> This is a ping of:
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00008.html
> 
> I have addressed the comments Jeff mentioned there.
> 
> This patch goes before the VR_VARYING + types patch, but I can adapt
> either one to come first.  I can even munge them together into one
> patch, if it helps review.
> 
> Tested on x86-64 Linux.
> 
> OK (assuming ChangeLog entries)?
> 
> Aldy
> 
> 
> curr.patch
> 
> commit ea908bdbfc8cdb4bb63e7d42630d01203edbac41
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Mon Jul 15 18:09:27 2019 +0200
> 
>     Enforce canonicalization in value_range.
Per your other message, I'll assume you'll write a suitable ChangeLog
entry before committing :-)


> 
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 594ee9adc17..de2f39d8487 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -69,23 +69,20 @@ along with GCC; see the file COPYING3.  If not see
>  #include "builtins.h"
>  #include "wide-int-range.h"
>  
> +static bool
> +ranges_from_anti_range (const value_range_base *ar,
> +			value_range_base *vr0, value_range_base *vr1,
> +			bool handle_pointers = false);
> +
Presumably this was better than moving the implementation earlier.


> +/* Normalize symbolics into constants.  */
> +
> +value_range_base
> +value_range_base::normalize_symbolics () const
> +{
> +  if (varying_p () || undefined_p ())
> +    return *this;
> +  tree ttype = type ();
> +  bool min_symbolic = !is_gimple_min_invariant (min ());
> +  bool max_symbolic = !is_gimple_min_invariant (max ());
Sadly "symbolic" is a particularly poor term.  When I think symbolics, I
think SYMBOL_REF, LABEL_REF and the tree equivalents.  They're
*symbols*.  Symbolics in this case are really things like SSA_NAMEs.
Confused the hell out of me briefly.

If we weren't on a path to kill VRP I'd probably suggest a distinct
effort to constify this code.  Some of the changes were a bit confusing
when it looked like we'd dropped a call to set the range of an object.
But those were just local copies, so setting the type/min/max directly
was actually fine.  constification would make this a bit clearer.  But
again, I don't think it's worth the effort given the long term
trajectory for tree-vrp.c.


So where does the handle_pointers stuff matter?   I'm a bit surprised we
have to do anything special for them.


OK.  I don't expect the answers to the minor questions above will
ultimately change anything.

jeff