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 <al...@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