This is the mail archive of the
mailing list for the GCC project.
Re: enforce canonicalization of value_range's
- From: Jeff Law <law at redhat dot com>
- To: Aldy Hernandez <aldyher at gmail dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Cc: Andrew MacLeod <amacleod at redhat dot com>
- Date: Mon, 12 Aug 2019 17:32:58 -0600
- Subject: Re: enforce canonicalization of value_range's
- References: <email@example.com>
On 8/12/19 12:48 PM, Aldy Hernandez wrote:
> This is a ping of:
> 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)?
> commit ea908bdbfc8cdb4bb63e7d42630d01203edbac41
> Author: Aldy Hernandez <firstname.lastname@example.org>
> 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::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.