Add value_range_base (w/o equivs)

Message ID alpine.LSU.2.20.1811091509460.1827@zhemvz.fhfr.qr
State New
Headers show
Series
  • Add value_range_base (w/o equivs)
Related show

Commit Message

Richard Biener Nov. 9, 2018, 2:19 p.m.
This adds value_range_base, a base class of class value_range
with all members but the m_equiv one.

I have looked into the sole GC user, IPA propagation, and replaced
the value_range use there with value_range_base since it also
asserted the equiv member is always NULL.

This in turn means I have written down that GC users only can
use value_range_base (and fixed the accessability issue with
adding a bunch of friends).

I have moved all methods that are required for this single user
sofar (without looking with others are trivially movable because
they do not look at equivs - that's for a followup).  I ended
up basically duplicating the ::union_ wrapper around union_ranges
(boo).  Some more refactoring might save us a few lines here.

There are two places where IPA prop calls extract_range_from_unary_expr.
I've temporarily made it use value_range temporaries there given
I need to think about the few cases of ->deep_copy () we have in
there.  IMHO equiv handling would need to move to the callers or
rather "copies" shouldn't be handled by extract_range_from_unary_expr.
Well, I need to think about it.  (no, I don't want to make that
function virtual)

The other workers might be even easier to massage to work on 
value_range_base only.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

If that goes well I want to apply this even in its incomplete form.
Unless there are any complaints?

Thanks,
Richard.

From 525e2e74bbb666399fafcffe8f17e928f45ca898 Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguenther@suse.de>
Date: Fri, 9 Nov 2018 15:00:50 +0100
Subject: [PATCH] add-value_range_base

	* tree-vrp.h (class value_range_base): New base class for
	value_range containing all but the m_equiv member.
	(dump_value_range_base): Add.
	(range_includes_zero_p): Work on value_range_base.
	* tree-vrp.c (value_range_base::set): Split out base handling
	from...
	(value_range::set): this.
	(value_range::set_equiv): New.
	(value_range_base::value_range_base): New constructors.
	(value_range_base::check): Split out base handling from...
	(value_range::check): this.
	(value_range::equal_p): Refactor in terms of
	ignore_equivs_equal_p which is now member of the base.
	(value_range_base::set_undefined): New.
	(value_range_base::set_varying): Likewise.
	(value_range_base::dump):Split out base handling from...
	(value_range::dump): this.
	(value_range_base::set_and_canonicalize): Split out base handling
	from...
	(value_range::set_and_canonicalize): this.
	(value_range_base::union_): New.

	* ipa-prop.h (struct ipa_jump_func): Use value_range_base *
	for m_vr.
	* ipa-cp.c (class ipcp_vr_lattice): Use value_range_base
	instead of value_range everywhere.
	(ipcp_vr_lattice::print): Use dump_value_range_base.
	(ipcp_vr_lattice::meet_with): Adjust.
	(ipcp_vr_lattice::meet_with_1): Likewise.
	(ipa_vr_operation_and_type_effects): Likewise.
	(propagate_vr_across_jump_function): Likewise.
	* ipa-prop.c (struct ipa_vr_ggc_hash_traits): Likewise.
	(ipa_get_value_range): Likewise.
	(ipa_set_jfunc_vr): Likewise.
	(ipa_compute_jump_functions_for_edge): Likewise.

Comments

Aldy Hernandez Nov. 9, 2018, 5:48 p.m. | #1
On 11/9/18 9:19 AM, Richard Biener wrote:
> 
> This adds value_range_base, a base class of class value_range
> with all members but the m_equiv one.

First of all, thanks so much for doing this!

> 
> I have looked into the sole GC user, IPA propagation, and replaced
> the value_range use there with value_range_base since it also
> asserted the equiv member is always NULL.
> 
> This in turn means I have written down that GC users only can
> use value_range_base (and fixed the accessability issue with
> adding a bunch of friends).

> +
>  /* Range of values that can be associated with an SSA_NAME after VRP
> -   has executed.  */
> -class GTY((for_user)) value_range
> +   has executed.  Note you may only use value_range_base with GC memory.  */
> +class GTY((for_user)) value_range_base
> +{

GC users cannot use the derived value_range?  Either way could you 
document the "why" this is the case above?

And thanks for adding those accessibility friends, they're a pain to get 
right.  It's a pity we have to go to such lengths to deal with this shit.

> 
> I have moved all methods that are required for this single user
> sofar (without looking with others are trivially movable because
> they do not look at equivs - that's for a followup).  I ended

That would be great.

> up basically duplicating the ::union_ wrapper around union_ranges
> (boo).  Some more refactoring might save us a few lines here.
> 
> There are two places where IPA prop calls extract_range_from_unary_expr.
> I've temporarily made it use value_range temporaries there given
> I need to think about the few cases of ->deep_copy () we have in
> there.  IMHO equiv handling would need to move to the callers or
> rather "copies" shouldn't be handled by extract_range_from_unary_expr.
> Well, I need to think about it.  (no, I don't want to make that
> function virtual)
> 
> The other workers might be even easier to massage to work on
> value_range_base only.

If value_range_base is the predominant idiom, perhaps it should be 
called value_range and the derived class be called 
value_range_with_equiv or some such?

> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> If that goes well I want to apply this even in its incomplete form.
> Unless there are any complaints?

Fine by me.  Again, thanks.
Aldy
Jeff Law Nov. 9, 2018, 8:59 p.m. | #2
On 11/9/18 7:19 AM, Richard Biener wrote:
> 
> This adds value_range_base, a base class of class value_range
> with all members but the m_equiv one.
> 
> I have looked into the sole GC user, IPA propagation, and replaced
> the value_range use there with value_range_base since it also
> asserted the equiv member is always NULL.
> 
> This in turn means I have written down that GC users only can
> use value_range_base (and fixed the accessability issue with
> adding a bunch of friends).
> 
> I have moved all methods that are required for this single user
> sofar (without looking with others are trivially movable because
> they do not look at equivs - that's for a followup).  I ended
> up basically duplicating the ::union_ wrapper around union_ranges
> (boo).  Some more refactoring might save us a few lines here.
> 
> There are two places where IPA prop calls extract_range_from_unary_expr.
> I've temporarily made it use value_range temporaries there given
> I need to think about the few cases of ->deep_copy () we have in
> there.  IMHO equiv handling would need to move to the callers or
> rather "copies" shouldn't be handled by extract_range_from_unary_expr.
> Well, I need to think about it.  (no, I don't want to make that
> function virtual)
> 
> The other workers might be even easier to massage to work on 
> value_range_base only.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> If that goes well I want to apply this even in its incomplete form.
> Unless there are any complaints?
> 
> Thanks,
> Richard.
> 
> From 525e2e74bbb666399fafcffe8f17e928f45ca898 Mon Sep 17 00:00:00 2001
> From: Richard Guenther <rguenther@suse.de>
> Date: Fri, 9 Nov 2018 15:00:50 +0100
> Subject: [PATCH] add-value_range_base
> 
> 	* tree-vrp.h (class value_range_base): New base class for
> 	value_range containing all but the m_equiv member.
> 	(dump_value_range_base): Add.
> 	(range_includes_zero_p): Work on value_range_base.
> 	* tree-vrp.c (value_range_base::set): Split out base handling
> 	from...
> 	(value_range::set): this.
> 	(value_range::set_equiv): New.
> 	(value_range_base::value_range_base): New constructors.
> 	(value_range_base::check): Split out base handling from...
> 	(value_range::check): this.
> 	(value_range::equal_p): Refactor in terms of
> 	ignore_equivs_equal_p which is now member of the base.
> 	(value_range_base::set_undefined): New.
> 	(value_range_base::set_varying): Likewise.
> 	(value_range_base::dump):Split out base handling from...
> 	(value_range::dump): this.
> 	(value_range_base::set_and_canonicalize): Split out base handling
> 	from...
> 	(value_range::set_and_canonicalize): this.
> 	(value_range_base::union_): New.
> 
> 	* ipa-prop.h (struct ipa_jump_func): Use value_range_base *
> 	for m_vr.
> 	* ipa-cp.c (class ipcp_vr_lattice): Use value_range_base
> 	instead of value_range everywhere.
> 	(ipcp_vr_lattice::print): Use dump_value_range_base.
> 	(ipcp_vr_lattice::meet_with): Adjust.
> 	(ipcp_vr_lattice::meet_with_1): Likewise.
> 	(ipa_vr_operation_and_type_effects): Likewise.
> 	(propagate_vr_across_jump_function): Likewise.
> 	* ipa-prop.c (struct ipa_vr_ggc_hash_traits): Likewise.
> 	(ipa_get_value_range): Likewise.
> 	(ipa_set_jfunc_vr): Likewise.
> 	(ipa_compute_jump_functions_for_edge): Likewise.
I like it.  Hell, I wish someone had suggested it last year, I could
have taken care of it then.

jeff
Richard Biener Nov. 11, 2018, 8:53 a.m. | #3
On Fri, 9 Nov 2018, Aldy Hernandez wrote:

> On 11/9/18 9:19 AM, Richard Biener wrote:
> > 
> > This adds value_range_base, a base class of class value_range
> > with all members but the m_equiv one.
> 
> First of all, thanks so much for doing this!
> 
> > 
> > I have looked into the sole GC user, IPA propagation, and replaced
> > the value_range use there with value_range_base since it also
> > asserted the equiv member is always NULL.
> > 
> > This in turn means I have written down that GC users only can
> > use value_range_base (and fixed the accessability issue with
> > adding a bunch of friends).
> 
> > +
> >  /* Range of values that can be associated with an SSA_NAME after VRP
> > -   has executed.  */
> > -class GTY((for_user)) value_range
> > +   has executed.  Note you may only use value_range_base with GC memory.
> > */
> > +class GTY((for_user)) value_range_base
> > +{
> 
> GC users cannot use the derived value_range?  Either way could you document
> the "why" this is the case above?

I've changed the comment as it was said to be confusing.  The reason is
the marking isn't implemented.

> And thanks for adding those accessibility friends, they're a pain to get
> right.  It's a pity we have to go to such lengths to deal with this shit.
> 
> > 
> > I have moved all methods that are required for this single user
> > sofar (without looking with others are trivially movable because
> > they do not look at equivs - that's for a followup).  I ended
> 
> That would be great.
> 
> > up basically duplicating the ::union_ wrapper around union_ranges
> > (boo).  Some more refactoring might save us a few lines here.
> > 
> > There are two places where IPA prop calls extract_range_from_unary_expr.
> > I've temporarily made it use value_range temporaries there given
> > I need to think about the few cases of ->deep_copy () we have in
> > there.  IMHO equiv handling would need to move to the callers or
> > rather "copies" shouldn't be handled by extract_range_from_unary_expr.
> > Well, I need to think about it.  (no, I don't want to make that
> > function virtual)
> > 
> > The other workers might be even easier to massage to work on
> > value_range_base only.
> 
> If value_range_base is the predominant idiom, perhaps it should be called
> value_range and the derived class be called value_range_with_equiv or some
> such?

Sure - the value_range_base approach was less intrusive.

> > 
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > 
> > If that goes well I want to apply this even in its incomplete form.
> > Unless there are any complaints?
> 
> Fine by me.  Again, thanks.

Installed as follows.  I'll see how I can complete it while
woring out of virtual office on Monday.

Richard.

2018-11-11  Richard Biener  <rguenther@suse.de>

	* tree-vrp.h (class value_range_base): New base class for
	value_range containing all but the m_equiv member.
	(dump_value_range_base): Add.
	(range_includes_zero_p): Work on value_range_base.
	* tree-vrp.c (value_range_base::set): Split out base handling
	from...
	(value_range::set): this.
	(value_range::set_equiv): New.
	(value_range_base::value_range_base): New constructors.
	(value_range_base::check): Split out base handling from...
	(value_range::check): this.
	(value_range::equal_p): Refactor in terms of
	ignore_equivs_equal_p which is now member of the base.
	(value_range_base::set_undefined): New.
	(value_range_base::set_varying): Likewise.
	(value_range_base::dump):Split out base handling from...
	(value_range::dump): this.
	(value_range_base::set_and_canonicalize): Split out base handling
	from...
	(value_range::set_and_canonicalize): this.
	(value_range_base::union_): New.

	* ipa-prop.h (struct ipa_jump_func): Use value_range_base *
	for m_vr.
	* ipa-cp.c (class ipcp_vr_lattice): Use value_range_base
	instead of value_range everywhere.
	(ipcp_vr_lattice::print): Use dump_value_range_base.
	(ipcp_vr_lattice::meet_with): Adjust.
	(ipcp_vr_lattice::meet_with_1): Likewise.
	(ipa_vr_operation_and_type_effects): Likewise.
	(propagate_vr_across_jump_function): Likewise.
	* ipa-prop.c (struct ipa_vr_ggc_hash_traits): Likewise.
	(ipa_get_value_range): Likewise.
	(ipa_set_jfunc_vr): Likewise.
	(ipa_compute_jump_functions_for_edge): Likewise.

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4471bae11c7..4f147eb37cc 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -307,18 +307,18 @@ private:
 class ipcp_vr_lattice
 {
 public:
-  value_range m_vr;
+  value_range_base m_vr;
 
   inline bool bottom_p () const;
   inline bool top_p () const;
   inline bool set_to_bottom ();
-  bool meet_with (const value_range *p_vr);
+  bool meet_with (const value_range_base *p_vr);
   bool meet_with (const ipcp_vr_lattice &other);
   void init () { gcc_assert (m_vr.undefined_p ()); }
   void print (FILE * f);
 
 private:
-  bool meet_with_1 (const value_range *other_vr);
+  bool meet_with_1 (const value_range_base *other_vr);
 };
 
 /* Structure containing lattices for a parameter itself and for pieces of
@@ -522,7 +522,7 @@ ipcp_bits_lattice::print (FILE *f)
 void
 ipcp_vr_lattice::print (FILE * f)
 {
-  dump_value_range (f, &m_vr);
+  dump_value_range_base (f, &m_vr);
 }
 
 /* Print all ipcp_lattices of all functions to F.  */
@@ -909,7 +909,7 @@ ipcp_vr_lattice::meet_with (const ipcp_vr_lattice &other)
    lattice.  */
 
 bool
-ipcp_vr_lattice::meet_with (const value_range *p_vr)
+ipcp_vr_lattice::meet_with (const value_range_base *p_vr)
 {
   return meet_with_1 (p_vr);
 }
@@ -918,7 +918,7 @@ ipcp_vr_lattice::meet_with (const value_range *p_vr)
    OTHER_VR lattice.  Return TRUE if anything changed.  */
 
 bool
-ipcp_vr_lattice::meet_with_1 (const value_range *other_vr)
+ipcp_vr_lattice::meet_with_1 (const value_range_base *other_vr)
 {
   if (bottom_p ())
     return false;
@@ -926,7 +926,7 @@ ipcp_vr_lattice::meet_with_1 (const value_range *other_vr)
   if (other_vr->varying_p ())
     return set_to_bottom ();
 
-  value_range save (m_vr);
+  value_range_base save (m_vr);
   m_vr.union_ (other_vr);
   return !m_vr.ignore_equivs_equal_p (save);
 }
@@ -1871,12 +1871,17 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
    the result is a range or an anti-range.  */
 
 static bool
-ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr,
+ipa_vr_operation_and_type_effects (value_range_base *dst_vr,
+				   value_range_base *src_vr,
 				   enum tree_code operation,
 				   tree dst_type, tree src_type)
 {
-  *dst_vr = value_range ();
-  extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type);
+  /* ???  We'd want to use value_range_base on the VRP workers.  */
+  value_range dst_tem;
+  value_range src_tem (*src_vr);
+  extract_range_from_unary_expr (&dst_tem, operation, dst_type,
+				 &src_tem, src_type);
+  *dst_vr = value_range_base (dst_tem.kind (), dst_tem.min (), dst_tem.max ());
   if (dst_vr->varying_p () || dst_vr->undefined_p ())
     return false;
   return true;
@@ -1915,7 +1920,7 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
 
 	  if (src_lats->m_value_range.bottom_p ())
 	    return dest_lat->set_to_bottom ();
-	  value_range vr;
+	  value_range_base vr;
 	  if (ipa_vr_operation_and_type_effects (&vr,
 						 &src_lats->m_value_range.m_vr,
 						 operation, param_type,
@@ -1932,12 +1937,12 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
 	  if (TREE_OVERFLOW_P (val))
 	    val = drop_tree_overflow (val);
 
-	  value_range tmpvr (VR_RANGE, val, val);
+	  value_range_base tmpvr (VR_RANGE, val, val);
 	  return dest_lat->meet_with (&tmpvr);
 	}
     }
 
-  value_range vr;
+  value_range_base vr;
   if (jfunc->m_vr
       && ipa_vr_operation_and_type_effects (&vr, jfunc->m_vr, NOP_EXPR,
 					    param_type,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 4bd0b4b4541..5d9d8cff52e 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -106,43 +106,42 @@ static GTY ((cache)) hash_table<ipa_bit_ggc_hash_traits> *ipa_bits_hash_table;
 /* Traits for a hash table for reusing value_ranges used for IPA.  Note that
    the equiv bitmap is not hashed and is expected to be NULL.  */
 
-struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range *>
+struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range_base *>
 {
-  typedef value_range *value_type;
-  typedef value_range *compare_type;
+  typedef value_range_base *value_type;
+  typedef value_range_base *compare_type;
   static hashval_t
-  hash (const value_range *p)
+  hash (const value_range_base *p)
     {
-      gcc_checking_assert (!p->equiv ());
       inchash::hash hstate (p->kind ());
       inchash::add_expr (p->min (), hstate);
       inchash::add_expr (p->max (), hstate);
       return hstate.end ();
     }
   static bool
-  equal (const value_range *a, const value_range *b)
+  equal (const value_range_base *a, const value_range_base *b)
     {
       return a->ignore_equivs_equal_p (*b);
     }
   static void
-  mark_empty (value_range *&p)
+  mark_empty (value_range_base *&p)
     {
       p = NULL;
     }
   static bool
-  is_empty (const value_range *p)
+  is_empty (const value_range_base *p)
     {
       return p == NULL;
     }
   static bool
-  is_deleted (const value_range *p)
+  is_deleted (const value_range_base *p)
     {
-      return p == reinterpret_cast<const value_range *> (1);
+      return p == reinterpret_cast<const value_range_base *> (1);
     }
   static void
-  mark_deleted (value_range *&p)
+  mark_deleted (value_range_base *&p)
     {
-      p = reinterpret_cast<value_range *> (1);
+      p = reinterpret_cast<value_range_base *> (1);
     }
 };
 
@@ -1770,14 +1769,14 @@ ipa_set_jfunc_bits (ipa_jump_func *jf, const widest_int &value,
 /* Return a pointer to a value_range just like *TMP, but either find it in
    ipa_vr_hash_table or allocate it in GC memory.  TMP->equiv must be NULL.  */
 
-static value_range *
-ipa_get_value_range (value_range *tmp)
+static value_range_base *
+ipa_get_value_range (value_range_base *tmp)
 {
-  value_range **slot = ipa_vr_hash_table->find_slot (tmp, INSERT);
+  value_range_base **slot = ipa_vr_hash_table->find_slot (tmp, INSERT);
   if (*slot)
     return *slot;
 
-  value_range *vr = ggc_alloc<value_range> ();
+  value_range_base *vr = ggc_alloc<value_range_base> ();
   *vr = *tmp;
   *slot = vr;
 
@@ -1788,10 +1787,10 @@ ipa_get_value_range (value_range *tmp)
    equiv set. Use hash table in order to avoid creating multiple same copies of
    value_ranges.  */
 
-static value_range *
+static value_range_base *
 ipa_get_value_range (enum value_range_kind type, tree min, tree max)
 {
-  value_range tmp (type, min, max);
+  value_range_base tmp (type, min, max);
   return ipa_get_value_range (&tmp);
 }
 
@@ -1810,7 +1809,7 @@ ipa_set_jfunc_vr (ipa_jump_func *jf, enum value_range_kind type,
    copy from ipa_vr_hash_table or allocate a new on in GC memory.  */
 
 static void
-ipa_set_jfunc_vr (ipa_jump_func *jf, value_range *tmp)
+ipa_set_jfunc_vr (ipa_jump_func *jf, value_range_base *tmp)
 {
   jf->m_vr = ipa_get_value_range (tmp);
 }
@@ -1886,6 +1885,8 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	      && (type = get_range_info (arg, &min, &max))
 	      && (type == VR_RANGE || type == VR_ANTI_RANGE))
 	    {
+	      /* ???  We'd want to use value_range_base here but the
+	         VRP workers need to be adjusted first.  */
 	      value_range resvr;
 	      value_range tmpvr (type,
 				 wide_int_to_tree (TREE_TYPE (arg), min),
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 8a53bb89f3f..5e826c5d3ba 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -182,7 +182,7 @@ struct GTY (()) ipa_jump_func
   /* Information about value range, containing valid data only when vr_known is
      true.  The pointed to structure is shared betweed different jump
      functions.  Use ipa_set_jfunc_vr to set this field.  */
-  struct value_range *m_vr;
+  struct value_range_base *m_vr;
 
   enum jump_func_type type;
   /* Represents a value of a jump function.  pass_through is used only in jump
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 73b9e047389..3ef676bb71b 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -73,16 +73,19 @@ along with GCC; see the file COPYING3.  If not see
    for still active basic-blocks.  */
 static sbitmap *live;
 
-/* Initialize value_range.  */
-
 void
-value_range::set (enum value_range_kind kind, tree min, tree max,
-		  bitmap equiv)
+value_range_base::set (enum value_range_kind kind, tree min, tree max)
 {
   m_kind = kind;
   m_min = min;
   m_max = max;
+  if (flag_checking)
+    check ();
+}
 
+void
+value_range::set_equiv (bitmap equiv)
+{
   /* Since updating the equivalence set involves deep copying the
      bitmaps, only do it if absolutely necessary.
 
@@ -99,10 +102,25 @@ value_range::set (enum value_range_kind kind, tree min, tree max,
       else
 	bitmap_clear (m_equiv);
     }
+}
+
+/* Initialize value_range.  */
+
+void
+value_range::set (enum value_range_kind kind, tree min, tree max,
+		  bitmap equiv)
+{
+  value_range_base::set (kind, min, max);
+  set_equiv (equiv);
   if (flag_checking)
     check ();
 }
 
+value_range_base::value_range_base (value_range_kind kind, tree min, tree max)
+{
+  set (kind, min, max);
+}
+
 value_range::value_range (value_range_kind kind, tree min, tree max,
 			  bitmap equiv)
 {
@@ -110,6 +128,12 @@ value_range::value_range (value_range_kind kind, tree min, tree max,
   set (kind, min, max, equiv);
 }
 
+value_range::value_range (const value_range_base &other)
+{
+  m_equiv = NULL;
+  set (other.kind (), other.min(), other.max (), NULL);
+}
+
 /* Like above, but keep the equivalences intact.  */
 
 void
@@ -133,7 +157,7 @@ value_range::deep_copy (const value_range *from)
 /* Check the validity of the range.  */
 
 void
-value_range::check ()
+value_range_base::check ()
 {
   switch (m_kind)
     {
@@ -158,22 +182,32 @@ value_range::check ()
     case VR_UNDEFINED:
     case VR_VARYING:
       gcc_assert (!min () && !max ());
-      gcc_assert (!m_equiv || bitmap_empty_p (m_equiv));
       break;
     default:
       gcc_unreachable ();
     }
 }
 
+void
+value_range::check ()
+{
+  value_range_base::check ();
+  switch (m_kind)
+    {
+    case VR_UNDEFINED:
+    case VR_VARYING:
+      gcc_assert (!m_equiv || bitmap_empty_p (m_equiv));
+    default:;
+    }
+}
+
 /* Returns TRUE if THIS == OTHER.  Ignores the equivalence bitmap if
    IGNORE_EQUIVS is TRUE.  */
 
 bool
 value_range::equal_p (const value_range &other, bool ignore_equivs) const
 {
-  return (m_kind == other.m_kind
-	  && vrp_operand_equal_p (m_min, other.m_min)
-	  && vrp_operand_equal_p (m_max, other.m_max)
+  return (ignore_equivs_equal_p (other)
 	  && (ignore_equivs
 	      || vrp_bitmap_equal_p (m_equiv, other.m_equiv)));
 }
@@ -181,9 +215,11 @@ value_range::equal_p (const value_range &other, bool ignore_equivs) const
 /* Return equality while ignoring equivalence bitmap.  */
 
 bool
-value_range::ignore_equivs_equal_p (const value_range &other) const
+value_range_base::ignore_equivs_equal_p (const value_range_base &other) const
 {
-  return equal_p (other, /*ignore_equivs=*/true);
+  return (m_kind == other.m_kind
+	  && vrp_operand_equal_p (m_min, other.m_min)
+	  && vrp_operand_equal_p (m_max, other.m_max));
 }
 
 bool
@@ -224,6 +260,12 @@ value_range::constant_p () const
 }
 
 void
+value_range_base::set_undefined ()
+{
+  *this = value_range_base (VR_UNDEFINED, NULL, NULL);
+}
+
+void
 value_range::set_undefined ()
 {
   equiv_clear ();
@@ -231,6 +273,12 @@ value_range::set_undefined ()
 }
 
 void
+value_range_base::set_varying ()
+{
+  *this = value_range_base (VR_VARYING, NULL, NULL);
+}
+
+void
 value_range::set_varying ()
 {
   equiv_clear ();
@@ -240,7 +288,7 @@ value_range::set_varying ()
 /* Return TRUE if it is possible that range contains VAL.  */
 
 bool
-value_range::may_contain_p (tree val) const
+value_range_base::may_contain_p (tree val) const
 {
   if (varying_p ())
     return true;
@@ -302,7 +350,7 @@ value_range::singleton_p (tree *result) const
 }
 
 tree
-value_range::type () const
+value_range_base::type () const
 {
   /* Types are only valid for VR_RANGE and VR_ANTI_RANGE, which are
      known to have non-zero min/max.  */
@@ -313,7 +361,7 @@ value_range::type () const
 /* Dump value range to FILE.  */
 
 void
-value_range::dump (FILE *file) const
+value_range_base::dump (FILE *file) const
 {
   if (undefined_p ())
     fprintf (file, "UNDEFINED");
@@ -339,23 +387,6 @@ value_range::dump (FILE *file) const
 	print_generic_expr (file, max ());
 
       fprintf (file, "]");
-
-      if (m_equiv)
-	{
-	  bitmap_iterator bi;
-	  unsigned i, c = 0;
-
-	  fprintf (file, "  EQUIVALENCES: { ");
-
-	  EXECUTE_IF_SET_IN_BITMAP (m_equiv, 0, i, bi)
-	    {
-	      print_generic_expr (file, ssa_name (i));
-	      fprintf (file, " ");
-	      c++;
-	    }
-
-	  fprintf (file, "} (%u elements)", c);
-	}
     }
   else if (varying_p ())
     fprintf (file, "VARYING");
@@ -364,6 +395,29 @@ value_range::dump (FILE *file) const
 }
 
 void
+value_range::dump (FILE *file) const
+{
+  value_range_base::dump (file);
+  if ((m_kind == VR_RANGE || m_kind == VR_ANTI_RANGE)
+      && m_equiv)
+    {
+      bitmap_iterator bi;
+      unsigned i, c = 0;
+
+      fprintf (file, "  EQUIVALENCES: { ");
+
+      EXECUTE_IF_SET_IN_BITMAP (m_equiv, 0, i, bi)
+	{
+	  print_generic_expr (file, ssa_name (i));
+	  fprintf (file, " ");
+	  c++;
+	}
+
+      fprintf (file, "} (%u elements)", c);
+    }
+}
+
+void
 value_range::dump () const
 {
   dump_value_range (stderr, this);
@@ -573,8 +627,8 @@ set_value_range (value_range *vr, enum value_range_kind kind,
    extract ranges from var + CST op limit.  */
 
 void
-value_range::set_and_canonicalize (enum value_range_kind kind,
-				   tree min, tree max, bitmap equiv)
+value_range_base::set_and_canonicalize (enum value_range_kind kind,
+					tree min, tree max)
 {
   /* Use the canonical setters for VR_UNDEFINED and VR_VARYING.  */
   if (kind == VR_UNDEFINED)
@@ -592,7 +646,7 @@ value_range::set_and_canonicalize (enum value_range_kind kind,
   if (TREE_CODE (min) != INTEGER_CST
       || TREE_CODE (max) != INTEGER_CST)
     {
-      set_value_range (this, kind, min, max, equiv);
+      set (kind, min, max);
       return;
     }
 
@@ -680,7 +734,18 @@ value_range::set_and_canonicalize (enum value_range_kind kind,
      to make sure VRP iteration terminates, otherwise we can get into
      oscillations.  */
 
-  set_value_range (this, kind, min, max, equiv);
+  set (kind, min, max);
+}
+
+void
+value_range::set_and_canonicalize (enum value_range_kind kind,
+				   tree min, tree max, bitmap equiv)
+{
+  value_range_base::set_and_canonicalize (kind, min, max);
+  if (this->kind () == VR_RANGE || this->kind () == VR_ANTI_RANGE)
+    set_equiv (equiv);
+  else
+    equiv_clear ();
 }
 
 /* Set value range VR to a single value.  This function is only called
@@ -1084,7 +1149,7 @@ value_inside_range (tree val, tree min, tree max)
 /* Return TRUE if *VR includes the value zero.  */
 
 bool
-range_includes_zero_p (const value_range *vr)
+range_includes_zero_p (const value_range_base *vr)
 {
   if (vr->varying_p () || vr->undefined_p ())
     return true;
@@ -2119,6 +2184,15 @@ dump_value_range (FILE *file, const value_range *vr)
     vr->dump (file);
 }
 
+void
+dump_value_range_base (FILE *file, const value_range_base *vr)
+{
+  if (!vr)
+    fprintf (file, "[]");
+  else
+    vr->dump (file);
+}
+
 /* Dump value range VR to stderr.  */
 
 DEBUG_FUNCTION void
@@ -6018,6 +6092,63 @@ value_range::intersect (const value_range *other)
    may not be the smallest possible such range.  */
 
 void
+value_range_base::union_ (const value_range_base *other)
+{
+  if (other->undefined_p ())
+    {
+      /* this already has the resulting range.  */
+      return;
+    }
+
+  if (this->undefined_p ())
+    {
+      *this = *other;
+      return;
+    }
+
+  if (this->varying_p ())
+    {
+      /* Nothing to do.  VR0 already has the resulting range.  */
+      return;
+    }
+
+  if (other->varying_p ())
+    {
+      this->set_varying ();
+      return;
+    }
+
+  value_range saved (*this);
+  value_range_kind vr0type = this->kind ();
+  tree vr0min = this->min ();
+  tree vr0max = this->max ();
+  union_ranges (&vr0type, &vr0min, &vr0max,
+		other->kind (), other->min (), other->max ());
+  *this = value_range_base (vr0type, vr0min, vr0max);
+  if (this->varying_p ())
+    {
+      /* Failed to find an efficient meet.  Before giving up and setting
+	 the result to VARYING, see if we can at least derive a useful
+	 anti-range.  */
+      if (range_includes_zero_p (&saved) == 0
+	  && range_includes_zero_p (other) == 0)
+	{
+	  tree zero = build_int_cst (saved.type (), 0);
+	  *this = value_range_base (VR_ANTI_RANGE, zero, zero);
+	  return;
+	}
+
+      this->set_varying ();
+      return;
+    }
+  this->set_and_canonicalize (this->kind (), this->min (), this->max ());
+}
+
+/* Meet operation for value ranges.  Given two value ranges VR0 and
+   VR1, store in VR0 a range that contains both VR0 and VR1.  This
+   may not be the smallest possible such range.  */
+
+void
 value_range::union_helper (value_range *vr0, const value_range *vr1)
 {
   if (vr1->undefined_p ())
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 3c870d52354..1e141c017e8 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -35,12 +35,63 @@ enum value_range_kind
   VR_LAST
 };
 
+
 /* Range of values that can be associated with an SSA_NAME after VRP
    has executed.  */
-class GTY((for_user)) value_range
+class GTY((for_user)) value_range_base
+{
+public:
+  value_range_base ();
+  value_range_base (value_range_kind, tree, tree);
+
+  enum value_range_kind kind () const;
+  tree min () const;
+  tree max () const;
+
+  /* Types of value ranges.  */
+  bool undefined_p () const;
+  bool varying_p () const;
+
+  void union_ (const value_range_base *);
+
+  bool ignore_equivs_equal_p (const value_range_base &) const;
+
+  void set_varying ();
+  void set_undefined ();
+
+  /* Misc methods.  */
+  tree type () const;
+  bool may_contain_p (tree) const;
+  void set_and_canonicalize (enum value_range_kind, tree, tree);
+
+  void dump (FILE *) const;
+
+protected:
+  void set (value_range_kind, tree, tree);
+  void check ();
+
+  enum value_range_kind m_kind;
+
+  tree m_min;
+  tree m_max;
+
+  friend void gt_ggc_mx_value_range_base (void *);
+  friend void gt_pch_p_16value_range_base (void *, void *,
+					   gt_pointer_operator, void *);
+  friend void gt_pch_nx_value_range_base (void *);
+  friend void gt_ggc_mx (value_range_base &);
+  friend void gt_ggc_mx (value_range_base *&);
+  friend void gt_pch_nx (value_range_base &);
+  friend void gt_pch_nx (value_range_base *, gt_pointer_operator, void *);
+};
+
+/* Note value_range cannot currently be used with GC memory, only
+   value_range_base is fully set up for this.  */
+class GTY((user)) value_range : public value_range_base
 {
  public:
   value_range ();
+  value_range (const value_range_base &);
   value_range (value_range_kind, tree, tree, bitmap = NULL);
   void update (value_range_kind, tree, tree);
   bool operator== (const value_range &) const;
@@ -49,8 +100,6 @@ class GTY((for_user)) value_range
   void union_ (const value_range *);
 
   /* Types of value ranges.  */
-  bool undefined_p () const;
-  bool varying_p () const;
   bool symbolic_p () const;
   bool constant_p () const;
   void set_undefined ();
@@ -62,49 +111,44 @@ class GTY((for_user)) value_range
   void equiv_add (const_tree, const value_range *, bitmap_obstack * = NULL);
 
   /* Misc methods.  */
-  tree type () const;
   bool zero_p () const;
-  bool may_contain_p (tree) const;
   bool singleton_p (tree *result = NULL) const;
   void deep_copy (const value_range *);
-  bool ignore_equivs_equal_p (const value_range &) const;
   void set_and_canonicalize (enum value_range_kind, tree, tree, bitmap);
   void dump (FILE *) const;
   void dump () const;
 
-  enum value_range_kind kind () const;
-  tree min () const;
-  tree max () const;
-
  private:
   void set (value_range_kind, tree, tree, bitmap);
+  void set_equiv (bitmap);
   void check ();
   bool equal_p (const value_range &, bool ignore_equivs) const;
   void intersect_helper (value_range *, const value_range *);
   void union_helper (value_range *, const value_range *);
 
-  enum value_range_kind m_kind;
- public:
-  /* These should be private, but GTY is a piece of crap.  */
-  tree m_min;
-  tree m_max;
   /* Set of SSA names whose value ranges are equivalent to this one.
      This set is only valid when TYPE is VR_RANGE or VR_ANTI_RANGE.  */
   bitmap m_equiv;
 };
 
 inline
-value_range::value_range ()
+value_range_base::value_range_base ()
 {
   m_kind = VR_UNDEFINED;
   m_min = m_max = NULL;
+}
+
+inline
+value_range::value_range ()
+  : value_range_base ()
+{
   m_equiv = NULL;
 }
 
 /* Return the kind of this range.  */
 
 inline value_range_kind
-value_range::kind () const
+value_range_base::kind () const
 {
   return m_kind;
 }
@@ -118,7 +162,7 @@ value_range::equiv () const
 /* Return the lower bound.  */
 
 inline tree
-value_range::min () const
+value_range_base::min () const
 {
   return m_min;
 }
@@ -126,7 +170,7 @@ value_range::min () const
 /* Return the upper bound.  */
 
 inline tree
-value_range::max () const
+value_range_base::max () const
 {
   return m_max;
 }
@@ -134,7 +178,7 @@ value_range::max () const
 /* Return TRUE if range spans the entire possible domain.  */
 
 inline bool
-value_range::varying_p () const
+value_range_base::varying_p () const
 {
   return m_kind == VR_VARYING;
 }
@@ -142,7 +186,7 @@ value_range::varying_p () const
 /* Return TRUE if range is undefined (essentially the empty set).  */
 
 inline bool
-value_range::undefined_p () const
+value_range_base::undefined_p () const
 {
   return m_kind == VR_UNDEFINED;
 }
@@ -158,6 +202,7 @@ value_range::zero_p () const
 }
 
 extern void dump_value_range (FILE *, const value_range *);
+extern void dump_value_range_base (FILE *, const value_range_base *);
 extern void extract_range_from_unary_expr (value_range *vr,
 					   enum tree_code code,
 					   tree type,
@@ -187,7 +232,7 @@ extern void register_edge_assert_for (tree, edge, enum tree_code,
 				      tree, tree, vec<assert_info> &);
 extern bool stmt_interesting_for_vrp (gimple *);
 extern void set_value_range_to_varying (value_range *);
-extern bool range_includes_zero_p (const value_range *);
+extern bool range_includes_zero_p (const value_range_base *);
 extern bool infer_value_range (gimple *, tree, tree_code *, tree *);
 
 extern void set_value_range_to_nonnull (value_range *, tree);
Aldy Hernandez Nov. 12, 2018, 10:35 a.m. | #4
On 11/11/18 3:53 AM, Richard Biener wrote:
> On Fri, 9 Nov 2018, Aldy Hernandez wrote:
> 
>> On 11/9/18 9:19 AM, Richard Biener wrote:
>>>
>>> This adds value_range_base, a base class of class value_range
>>> with all members but the m_equiv one.
>>
>> First of all, thanks so much for doing this!
>>
>>>
>>> I have looked into the sole GC user, IPA propagation, and replaced
>>> the value_range use there with value_range_base since it also
>>> asserted the equiv member is always NULL.
>>>
>>> This in turn means I have written down that GC users only can
>>> use value_range_base (and fixed the accessability issue with
>>> adding a bunch of friends).
>>
>>> +
>>>   /* Range of values that can be associated with an SSA_NAME after VRP
>>> -   has executed.  */
>>> -class GTY((for_user)) value_range
>>> +   has executed.  Note you may only use value_range_base with GC memory.
>>> */
>>> +class GTY((for_user)) value_range_base
>>> +{
>>
>> GC users cannot use the derived value_range?  Either way could you document
>> the "why" this is the case above?
> 
> I've changed the comment as it was said to be confusing.  The reason is
> the marking isn't implemented.

Ah, I see.  In which case, shouldn't you then remove the GTY() markers 
from the derived class?

/* Note value_range cannot currently be used with GC memory, only
    value_range_base is fully set up for this.  */
class GTY((user)) value_range : public value_range_base
Richard Biener Nov. 12, 2018, 11:10 a.m. | #5
On Mon, 12 Nov 2018, Aldy Hernandez wrote:

> 
> 
> On 11/11/18 3:53 AM, Richard Biener wrote:
> > On Fri, 9 Nov 2018, Aldy Hernandez wrote:
> > 
> > > On 11/9/18 9:19 AM, Richard Biener wrote:
> > > > 
> > > > This adds value_range_base, a base class of class value_range
> > > > with all members but the m_equiv one.
> > > 
> > > First of all, thanks so much for doing this!
> > > 
> > > > 
> > > > I have looked into the sole GC user, IPA propagation, and replaced
> > > > the value_range use there with value_range_base since it also
> > > > asserted the equiv member is always NULL.
> > > > 
> > > > This in turn means I have written down that GC users only can
> > > > use value_range_base (and fixed the accessability issue with
> > > > adding a bunch of friends).
> > > 
> > > > +
> > > >   /* Range of values that can be associated with an SSA_NAME after VRP
> > > > -   has executed.  */
> > > > -class GTY((for_user)) value_range
> > > > +   has executed.  Note you may only use value_range_base with GC
> > > > memory.
> > > > */
> > > > +class GTY((for_user)) value_range_base
> > > > +{
> > > 
> > > GC users cannot use the derived value_range?  Either way could you
> > > document
> > > the "why" this is the case above?
> > 
> > I've changed the comment as it was said to be confusing.  The reason is
> > the marking isn't implemented.
> 
> Ah, I see.  In which case, shouldn't you then remove the GTY() markers from
> the derived class?
> 
> /* Note value_range cannot currently be used with GC memory, only
>    value_range_base is fully set up for this.  */
> class GTY((user)) value_range : public value_range_base

It's required to make gengtype happy...

Richard.
Aldy Hernandez Nov. 12, 2018, 11:11 a.m. | #6
Ughhhh ok.

On Mon, Nov 12, 2018, 12:10 Richard Biener <rguenther@suse.de wrote:

> On Mon, 12 Nov 2018, Aldy Hernandez wrote:
>
> >
> >
> > On 11/11/18 3:53 AM, Richard Biener wrote:
> > > On Fri, 9 Nov 2018, Aldy Hernandez wrote:
> > >
> > > > On 11/9/18 9:19 AM, Richard Biener wrote:
> > > > >
> > > > > This adds value_range_base, a base class of class value_range
> > > > > with all members but the m_equiv one.
> > > >
> > > > First of all, thanks so much for doing this!
> > > >
> > > > >
> > > > > I have looked into the sole GC user, IPA propagation, and replaced
> > > > > the value_range use there with value_range_base since it also
> > > > > asserted the equiv member is always NULL.
> > > > >
> > > > > This in turn means I have written down that GC users only can
> > > > > use value_range_base (and fixed the accessability issue with
> > > > > adding a bunch of friends).
> > > >
> > > > > +
> > > > >   /* Range of values that can be associated with an SSA_NAME after
> VRP
> > > > > -   has executed.  */
> > > > > -class GTY((for_user)) value_range
> > > > > +   has executed.  Note you may only use value_range_base with GC
> > > > > memory.
> > > > > */
> > > > > +class GTY((for_user)) value_range_base
> > > > > +{
> > > >
> > > > GC users cannot use the derived value_range?  Either way could you
> > > > document
> > > > the "why" this is the case above?
> > >
> > > I've changed the comment as it was said to be confusing.  The reason is
> > > the marking isn't implemented.
> >
> > Ah, I see.  In which case, shouldn't you then remove the GTY() markers
> from
> > the derived class?
> >
> > /* Note value_range cannot currently be used with GC memory, only
> >    value_range_base is fully set up for this.  */
> > class GTY((user)) value_range : public value_range_base
>
> It's required to make gengtype happy...
>
> Richard.
>

Patch

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4471bae11c7..4f147eb37cc 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -307,18 +307,18 @@  private:
 class ipcp_vr_lattice
 {
 public:
-  value_range m_vr;
+  value_range_base m_vr;
 
   inline bool bottom_p () const;
   inline bool top_p () const;
   inline bool set_to_bottom ();
-  bool meet_with (const value_range *p_vr);
+  bool meet_with (const value_range_base *p_vr);
   bool meet_with (const ipcp_vr_lattice &other);
   void init () { gcc_assert (m_vr.undefined_p ()); }
   void print (FILE * f);
 
 private:
-  bool meet_with_1 (const value_range *other_vr);
+  bool meet_with_1 (const value_range_base *other_vr);
 };
 
 /* Structure containing lattices for a parameter itself and for pieces of
@@ -522,7 +522,7 @@  ipcp_bits_lattice::print (FILE *f)
 void
 ipcp_vr_lattice::print (FILE * f)
 {
-  dump_value_range (f, &m_vr);
+  dump_value_range_base (f, &m_vr);
 }
 
 /* Print all ipcp_lattices of all functions to F.  */
@@ -909,7 +909,7 @@  ipcp_vr_lattice::meet_with (const ipcp_vr_lattice &other)
    lattice.  */
 
 bool
-ipcp_vr_lattice::meet_with (const value_range *p_vr)
+ipcp_vr_lattice::meet_with (const value_range_base *p_vr)
 {
   return meet_with_1 (p_vr);
 }
@@ -918,7 +918,7 @@  ipcp_vr_lattice::meet_with (const value_range *p_vr)
    OTHER_VR lattice.  Return TRUE if anything changed.  */
 
 bool
-ipcp_vr_lattice::meet_with_1 (const value_range *other_vr)
+ipcp_vr_lattice::meet_with_1 (const value_range_base *other_vr)
 {
   if (bottom_p ())
     return false;
@@ -926,7 +926,7 @@  ipcp_vr_lattice::meet_with_1 (const value_range *other_vr)
   if (other_vr->varying_p ())
     return set_to_bottom ();
 
-  value_range save (m_vr);
+  value_range_base save (m_vr);
   m_vr.union_ (other_vr);
   return !m_vr.ignore_equivs_equal_p (save);
 }
@@ -1871,12 +1871,17 @@  propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
    the result is a range or an anti-range.  */
 
 static bool
-ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr,
+ipa_vr_operation_and_type_effects (value_range_base *dst_vr,
+				   value_range_base *src_vr,
 				   enum tree_code operation,
 				   tree dst_type, tree src_type)
 {
-  *dst_vr = value_range ();
-  extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type);
+  /* ???  We'd want to use value_range_base on the VRP workers.  */
+  value_range dst_tem;
+  value_range src_tem (*src_vr);
+  extract_range_from_unary_expr (&dst_tem, operation, dst_type,
+				 &src_tem, src_type);
+  *dst_vr = value_range_base (dst_tem.kind (), dst_tem.min (), dst_tem.max ());
   if (dst_vr->varying_p () || dst_vr->undefined_p ())
     return false;
   return true;
@@ -1915,7 +1920,7 @@  propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
 
 	  if (src_lats->m_value_range.bottom_p ())
 	    return dest_lat->set_to_bottom ();
-	  value_range vr;
+	  value_range_base vr;
 	  if (ipa_vr_operation_and_type_effects (&vr,
 						 &src_lats->m_value_range.m_vr,
 						 operation, param_type,
@@ -1932,12 +1937,12 @@  propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
 	  if (TREE_OVERFLOW_P (val))
 	    val = drop_tree_overflow (val);
 
-	  value_range tmpvr (VR_RANGE, val, val);
+	  value_range_base tmpvr (VR_RANGE, val, val);
 	  return dest_lat->meet_with (&tmpvr);
 	}
     }
 
-  value_range vr;
+  value_range_base vr;
   if (jfunc->m_vr
       && ipa_vr_operation_and_type_effects (&vr, jfunc->m_vr, NOP_EXPR,
 					    param_type,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 4bd0b4b4541..5d9d8cff52e 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -106,43 +106,42 @@  static GTY ((cache)) hash_table<ipa_bit_ggc_hash_traits> *ipa_bits_hash_table;
 /* Traits for a hash table for reusing value_ranges used for IPA.  Note that
    the equiv bitmap is not hashed and is expected to be NULL.  */
 
-struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range *>
+struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range_base *>
 {
-  typedef value_range *value_type;
-  typedef value_range *compare_type;
+  typedef value_range_base *value_type;
+  typedef value_range_base *compare_type;
   static hashval_t
-  hash (const value_range *p)
+  hash (const value_range_base *p)
     {
-      gcc_checking_assert (!p->equiv ());
       inchash::hash hstate (p->kind ());
       inchash::add_expr (p->min (), hstate);
       inchash::add_expr (p->max (), hstate);
       return hstate.end ();
     }
   static bool
-  equal (const value_range *a, const value_range *b)
+  equal (const value_range_base *a, const value_range_base *b)
     {
       return a->ignore_equivs_equal_p (*b);
     }
   static void
-  mark_empty (value_range *&p)
+  mark_empty (value_range_base *&p)
     {
       p = NULL;
     }
   static bool
-  is_empty (const value_range *p)
+  is_empty (const value_range_base *p)
     {
       return p == NULL;
     }
   static bool
-  is_deleted (const value_range *p)
+  is_deleted (const value_range_base *p)
     {
-      return p == reinterpret_cast<const value_range *> (1);
+      return p == reinterpret_cast<const value_range_base *> (1);
     }
   static void
-  mark_deleted (value_range *&p)
+  mark_deleted (value_range_base *&p)
     {
-      p = reinterpret_cast<value_range *> (1);
+      p = reinterpret_cast<value_range_base *> (1);
     }
 };
 
@@ -1770,14 +1769,14 @@  ipa_set_jfunc_bits (ipa_jump_func *jf, const widest_int &value,
 /* Return a pointer to a value_range just like *TMP, but either find it in
    ipa_vr_hash_table or allocate it in GC memory.  TMP->equiv must be NULL.  */
 
-static value_range *
-ipa_get_value_range (value_range *tmp)
+static value_range_base *
+ipa_get_value_range (value_range_base *tmp)
 {
-  value_range **slot = ipa_vr_hash_table->find_slot (tmp, INSERT);
+  value_range_base **slot = ipa_vr_hash_table->find_slot (tmp, INSERT);
   if (*slot)
     return *slot;
 
-  value_range *vr = ggc_alloc<value_range> ();
+  value_range_base *vr = ggc_alloc<value_range_base> ();
   *vr = *tmp;
   *slot = vr;
 
@@ -1788,10 +1787,10 @@  ipa_get_value_range (value_range *tmp)
    equiv set. Use hash table in order to avoid creating multiple same copies of
    value_ranges.  */
 
-static value_range *
+static value_range_base *
 ipa_get_value_range (enum value_range_kind type, tree min, tree max)
 {
-  value_range tmp (type, min, max);
+  value_range_base tmp (type, min, max);
   return ipa_get_value_range (&tmp);
 }
 
@@ -1810,7 +1809,7 @@  ipa_set_jfunc_vr (ipa_jump_func *jf, enum value_range_kind type,
    copy from ipa_vr_hash_table or allocate a new on in GC memory.  */
 
 static void
-ipa_set_jfunc_vr (ipa_jump_func *jf, value_range *tmp)
+ipa_set_jfunc_vr (ipa_jump_func *jf, value_range_base *tmp)
 {
   jf->m_vr = ipa_get_value_range (tmp);
 }
@@ -1886,6 +1885,8 @@  ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	      && (type = get_range_info (arg, &min, &max))
 	      && (type == VR_RANGE || type == VR_ANTI_RANGE))
 	    {
+	      /* ???  We'd want to use value_range_base here but the
+	         VRP workers need to be adjusted first.  */
 	      value_range resvr;
 	      value_range tmpvr (type,
 				 wide_int_to_tree (TREE_TYPE (arg), min),
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 8a53bb89f3f..5e826c5d3ba 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -182,7 +182,7 @@  struct GTY (()) ipa_jump_func
   /* Information about value range, containing valid data only when vr_known is
      true.  The pointed to structure is shared betweed different jump
      functions.  Use ipa_set_jfunc_vr to set this field.  */
-  struct value_range *m_vr;
+  struct value_range_base *m_vr;
 
   enum jump_func_type type;
   /* Represents a value of a jump function.  pass_through is used only in jump
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 73b9e047389..79a583b90a1 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -73,16 +73,19 @@  along with GCC; see the file COPYING3.  If not see
    for still active basic-blocks.  */
 static sbitmap *live;
 
-/* Initialize value_range.  */
-
 void
-value_range::set (enum value_range_kind kind, tree min, tree max,
-		  bitmap equiv)
+value_range_base::set (enum value_range_kind kind, tree min, tree max)
 {
   m_kind = kind;
   m_min = min;
   m_max = max;
+  if (flag_checking)
+    check ();
+}
 
+void
+value_range::set_equiv (bitmap equiv)
+{
   /* Since updating the equivalence set involves deep copying the
      bitmaps, only do it if absolutely necessary.
 
@@ -99,10 +102,25 @@  value_range::set (enum value_range_kind kind, tree min, tree max,
       else
 	bitmap_clear (m_equiv);
     }
+}
+
+/* Initialize value_range.  */
+
+void
+value_range::set (enum value_range_kind kind, tree min, tree max,
+		  bitmap equiv)
+{
+  value_range_base::set (kind, min, max);
+  set_equiv (equiv);
   if (flag_checking)
     check ();
 }
 
+value_range_base::value_range_base (value_range_kind kind, tree min, tree max)
+{
+  set (kind, min, max);
+}
+
 value_range::value_range (value_range_kind kind, tree min, tree max,
 			  bitmap equiv)
 {
@@ -110,6 +128,12 @@  value_range::value_range (value_range_kind kind, tree min, tree max,
   set (kind, min, max, equiv);
 }
 
+value_range::value_range (const value_range_base &other)
+{
+  m_equiv = NULL;
+  set (other.kind (), other.min(), other.max (), NULL);
+}
+
 /* Like above, but keep the equivalences intact.  */
 
 void
@@ -133,7 +157,7 @@  value_range::deep_copy (const value_range *from)
 /* Check the validity of the range.  */
 
 void
-value_range::check ()
+value_range_base::check ()
 {
   switch (m_kind)
     {
@@ -158,22 +182,32 @@  value_range::check ()
     case VR_UNDEFINED:
     case VR_VARYING:
       gcc_assert (!min () && !max ());
-      gcc_assert (!m_equiv || bitmap_empty_p (m_equiv));
       break;
     default:
       gcc_unreachable ();
     }
 }
 
+void
+value_range::check ()
+{
+  value_range_base::check ();
+  switch (m_kind)
+    {
+    case VR_UNDEFINED:
+    case VR_VARYING:
+      gcc_assert (!m_equiv || bitmap_empty_p (m_equiv));
+    default:;
+    }
+}
+
 /* Returns TRUE if THIS == OTHER.  Ignores the equivalence bitmap if
    IGNORE_EQUIVS is TRUE.  */
 
 bool
 value_range::equal_p (const value_range &other, bool ignore_equivs) const
 {
-  return (m_kind == other.m_kind
-	  && vrp_operand_equal_p (m_min, other.m_min)
-	  && vrp_operand_equal_p (m_max, other.m_max)
+  return (ignore_equivs_equal_p (other)
 	  && (ignore_equivs
 	      || vrp_bitmap_equal_p (m_equiv, other.m_equiv)));
 }
@@ -181,9 +215,11 @@  value_range::equal_p (const value_range &other, bool ignore_equivs) const
 /* Return equality while ignoring equivalence bitmap.  */
 
 bool
-value_range::ignore_equivs_equal_p (const value_range &other) const
+value_range_base::ignore_equivs_equal_p (const value_range_base &other) const
 {
-  return equal_p (other, /*ignore_equivs=*/true);
+  return (m_kind == other.m_kind
+	  && vrp_operand_equal_p (m_min, other.m_min)
+	  && vrp_operand_equal_p (m_max, other.m_max));
 }
 
 bool
@@ -224,6 +260,12 @@  value_range::constant_p () const
 }
 
 void
+value_range_base::set_undefined ()
+{
+  *this = value_range_base (VR_UNDEFINED, NULL, NULL);
+}
+
+void
 value_range::set_undefined ()
 {
   equiv_clear ();
@@ -231,6 +273,12 @@  value_range::set_undefined ()
 }
 
 void
+value_range_base::set_varying ()
+{
+  *this = value_range_base (VR_VARYING, NULL, NULL);
+}
+
+void
 value_range::set_varying ()
 {
   equiv_clear ();
@@ -240,7 +288,7 @@  value_range::set_varying ()
 /* Return TRUE if it is possible that range contains VAL.  */
 
 bool
-value_range::may_contain_p (tree val) const
+value_range_base::may_contain_p (tree val) const
 {
   if (varying_p ())
     return true;
@@ -302,7 +350,7 @@  value_range::singleton_p (tree *result) const
 }
 
 tree
-value_range::type () const
+value_range_base::type () const
 {
   /* Types are only valid for VR_RANGE and VR_ANTI_RANGE, which are
      known to have non-zero min/max.  */
@@ -313,7 +361,7 @@  value_range::type () const
 /* Dump value range to FILE.  */
 
 void
-value_range::dump (FILE *file) const
+value_range_base::dump (FILE *file) const
 {
   if (undefined_p ())
     fprintf (file, "UNDEFINED");
@@ -339,23 +387,6 @@  value_range::dump (FILE *file) const
 	print_generic_expr (file, max ());
 
       fprintf (file, "]");
-
-      if (m_equiv)
-	{
-	  bitmap_iterator bi;
-	  unsigned i, c = 0;
-
-	  fprintf (file, "  EQUIVALENCES: { ");
-
-	  EXECUTE_IF_SET_IN_BITMAP (m_equiv, 0, i, bi)
-	    {
-	      print_generic_expr (file, ssa_name (i));
-	      fprintf (file, " ");
-	      c++;
-	    }
-
-	  fprintf (file, "} (%u elements)", c);
-	}
     }
   else if (varying_p ())
     fprintf (file, "VARYING");
@@ -364,6 +395,29 @@  value_range::dump (FILE *file) const
 }
 
 void
+value_range::dump (FILE *file) const
+{
+  value_range_base::dump (file);
+  if ((m_kind == VR_RANGE || m_kind == VR_ANTI_RANGE)
+      && m_equiv)
+    {
+      bitmap_iterator bi;
+      unsigned i, c = 0;
+
+      fprintf (file, "  EQUIVALENCES: { ");
+
+      EXECUTE_IF_SET_IN_BITMAP (m_equiv, 0, i, bi)
+	{
+	  print_generic_expr (file, ssa_name (i));
+	  fprintf (file, " ");
+	  c++;
+	}
+
+      fprintf (file, "} (%u elements)", c);
+    }
+}
+
+void
 value_range::dump () const
 {
   dump_value_range (stderr, this);
@@ -573,8 +627,8 @@  set_value_range (value_range *vr, enum value_range_kind kind,
    extract ranges from var + CST op limit.  */
 
 void
-value_range::set_and_canonicalize (enum value_range_kind kind,
-				   tree min, tree max, bitmap equiv)
+value_range_base::set_and_canonicalize (enum value_range_kind kind,
+					tree min, tree max)
 {
   /* Use the canonical setters for VR_UNDEFINED and VR_VARYING.  */
   if (kind == VR_UNDEFINED)
@@ -592,7 +646,7 @@  value_range::set_and_canonicalize (enum value_range_kind kind,
   if (TREE_CODE (min) != INTEGER_CST
       || TREE_CODE (max) != INTEGER_CST)
     {
-      set_value_range (this, kind, min, max, equiv);
+      set (kind, min, max);
       return;
     }
 
@@ -680,7 +734,16 @@  value_range::set_and_canonicalize (enum value_range_kind kind,
      to make sure VRP iteration terminates, otherwise we can get into
      oscillations.  */
 
-  set_value_range (this, kind, min, max, equiv);
+  set (kind, min, max);
+}
+
+void
+value_range::set_and_canonicalize (enum value_range_kind kind,
+				   tree min, tree max, bitmap equiv)
+{
+  value_range_base::set_and_canonicalize (kind, min, max);
+  if (this->kind () == VR_RANGE || this->kind () == VR_ANTI_RANGE)
+    set_equiv (equiv);
 }
 
 /* Set value range VR to a single value.  This function is only called
@@ -1084,7 +1147,7 @@  value_inside_range (tree val, tree min, tree max)
 /* Return TRUE if *VR includes the value zero.  */
 
 bool
-range_includes_zero_p (const value_range *vr)
+range_includes_zero_p (const value_range_base *vr)
 {
   if (vr->varying_p () || vr->undefined_p ())
     return true;
@@ -2119,6 +2182,15 @@  dump_value_range (FILE *file, const value_range *vr)
     vr->dump (file);
 }
 
+void
+dump_value_range_base (FILE *file, const value_range_base *vr)
+{
+  if (!vr)
+    fprintf (file, "[]");
+  else
+    vr->dump (file);
+}
+
 /* Dump value range VR to stderr.  */
 
 DEBUG_FUNCTION void
@@ -6018,6 +6090,63 @@  value_range::intersect (const value_range *other)
    may not be the smallest possible such range.  */
 
 void
+value_range_base::union_ (const value_range_base *other)
+{
+  if (other->undefined_p ())
+    {
+      /* this already has the resulting range.  */
+      return;
+    }
+
+  if (this->undefined_p ())
+    {
+      *this = *other;
+      return;
+    }
+
+  if (this->varying_p ())
+    {
+      /* Nothing to do.  VR0 already has the resulting range.  */
+      return;
+    }
+
+  if (other->varying_p ())
+    {
+      this->set_varying ();
+      return;
+    }
+
+  value_range saved (*this);
+  value_range_kind vr0type = this->kind ();
+  tree vr0min = this->min ();
+  tree vr0max = this->max ();
+  union_ranges (&vr0type, &vr0min, &vr0max,
+		other->kind (), other->min (), other->max ());
+  *this = value_range_base (vr0type, vr0min, vr0max);
+  if (this->varying_p ())
+    {
+      /* Failed to find an efficient meet.  Before giving up and setting
+	 the result to VARYING, see if we can at least derive a useful
+	 anti-range.  */
+      if (range_includes_zero_p (&saved) == 0
+	  && range_includes_zero_p (other) == 0)
+	{
+	  tree zero = build_int_cst (saved.type (), 0);
+	  *this = value_range_base (VR_ANTI_RANGE, zero, zero);
+	  return;
+	}
+
+      this->set_varying ();
+      return;
+    }
+  this->set_and_canonicalize (this->kind (), this->min (), this->max ());
+}
+
+/* Meet operation for value ranges.  Given two value ranges VR0 and
+   VR1, store in VR0 a range that contains both VR0 and VR1.  This
+   may not be the smallest possible such range.  */
+
+void
 value_range::union_helper (value_range *vr0, const value_range *vr1)
 {
   if (vr1->undefined_p ())
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 3c870d52354..4bc956f76cb 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -35,12 +35,61 @@  enum value_range_kind
   VR_LAST
 };
 
+
 /* Range of values that can be associated with an SSA_NAME after VRP
-   has executed.  */
-class GTY((for_user)) value_range
+   has executed.  Note you may only use value_range_base with GC memory.  */
+class GTY((for_user)) value_range_base
+{
+public:
+  value_range_base ();
+  value_range_base (value_range_kind, tree, tree);
+
+  enum value_range_kind kind () const;
+  tree min () const;
+  tree max () const;
+
+  /* Types of value ranges.  */
+  bool undefined_p () const;
+  bool varying_p () const;
+
+  void union_ (const value_range_base *);
+
+  bool ignore_equivs_equal_p (const value_range_base &) const;
+
+  void set_varying ();
+  void set_undefined ();
+
+  /* Misc methods.  */
+  tree type () const;
+  bool may_contain_p (tree) const;
+  void set_and_canonicalize (enum value_range_kind, tree, tree);
+
+  void dump (FILE *) const;
+
+protected:
+  void set (value_range_kind, tree, tree);
+  void check ();
+
+  enum value_range_kind m_kind;
+
+  tree m_min;
+  tree m_max;
+
+  friend void gt_ggc_mx_value_range_base (void *);
+  friend void gt_pch_p_16value_range_base (void *, void *,
+					   gt_pointer_operator, void *);
+  friend void gt_pch_nx_value_range_base (void *);
+  friend void gt_ggc_mx (value_range_base &);
+  friend void gt_ggc_mx (value_range_base *&);
+  friend void gt_pch_nx (value_range_base &);
+  friend void gt_pch_nx (value_range_base *, gt_pointer_operator, void *);
+};
+
+class GTY((user)) value_range : public value_range_base
 {
  public:
   value_range ();
+  value_range (const value_range_base &);
   value_range (value_range_kind, tree, tree, bitmap = NULL);
   void update (value_range_kind, tree, tree);
   bool operator== (const value_range &) const;
@@ -49,8 +98,6 @@  class GTY((for_user)) value_range
   void union_ (const value_range *);
 
   /* Types of value ranges.  */
-  bool undefined_p () const;
-  bool varying_p () const;
   bool symbolic_p () const;
   bool constant_p () const;
   void set_undefined ();
@@ -62,49 +109,44 @@  class GTY((for_user)) value_range
   void equiv_add (const_tree, const value_range *, bitmap_obstack * = NULL);
 
   /* Misc methods.  */
-  tree type () const;
   bool zero_p () const;
-  bool may_contain_p (tree) const;
   bool singleton_p (tree *result = NULL) const;
   void deep_copy (const value_range *);
-  bool ignore_equivs_equal_p (const value_range &) const;
   void set_and_canonicalize (enum value_range_kind, tree, tree, bitmap);
   void dump (FILE *) const;
   void dump () const;
 
-  enum value_range_kind kind () const;
-  tree min () const;
-  tree max () const;
-
  private:
   void set (value_range_kind, tree, tree, bitmap);
+  void set_equiv (bitmap);
   void check ();
   bool equal_p (const value_range &, bool ignore_equivs) const;
   void intersect_helper (value_range *, const value_range *);
   void union_helper (value_range *, const value_range *);
 
-  enum value_range_kind m_kind;
- public:
-  /* These should be private, but GTY is a piece of crap.  */
-  tree m_min;
-  tree m_max;
   /* Set of SSA names whose value ranges are equivalent to this one.
      This set is only valid when TYPE is VR_RANGE or VR_ANTI_RANGE.  */
   bitmap m_equiv;
 };
 
 inline
-value_range::value_range ()
+value_range_base::value_range_base ()
 {
   m_kind = VR_UNDEFINED;
   m_min = m_max = NULL;
+}
+
+inline
+value_range::value_range ()
+  : value_range_base ()
+{
   m_equiv = NULL;
 }
 
 /* Return the kind of this range.  */
 
 inline value_range_kind
-value_range::kind () const
+value_range_base::kind () const
 {
   return m_kind;
 }
@@ -118,7 +160,7 @@  value_range::equiv () const
 /* Return the lower bound.  */
 
 inline tree
-value_range::min () const
+value_range_base::min () const
 {
   return m_min;
 }
@@ -126,7 +168,7 @@  value_range::min () const
 /* Return the upper bound.  */
 
 inline tree
-value_range::max () const
+value_range_base::max () const
 {
   return m_max;
 }
@@ -134,7 +176,7 @@  value_range::max () const
 /* Return TRUE if range spans the entire possible domain.  */
 
 inline bool
-value_range::varying_p () const
+value_range_base::varying_p () const
 {
   return m_kind == VR_VARYING;
 }
@@ -142,7 +184,7 @@  value_range::varying_p () const
 /* Return TRUE if range is undefined (essentially the empty set).  */
 
 inline bool
-value_range::undefined_p () const
+value_range_base::undefined_p () const
 {
   return m_kind == VR_UNDEFINED;
 }
@@ -158,6 +200,7 @@  value_range::zero_p () const
 }
 
 extern void dump_value_range (FILE *, const value_range *);
+extern void dump_value_range_base (FILE *, const value_range_base *);
 extern void extract_range_from_unary_expr (value_range *vr,
 					   enum tree_code code,
 					   tree type,
@@ -187,7 +230,7 @@  extern void register_edge_assert_for (tree, edge, enum tree_code,
 				      tree, tree, vec<assert_info> &);
 extern bool stmt_interesting_for_vrp (gimple *);
 extern void set_value_range_to_varying (value_range *);
-extern bool range_includes_zero_p (const value_range *);
+extern bool range_includes_zero_p (const value_range_base *);
 extern bool infer_value_range (gimple *, tree, tree_code *, tree *);
 
 extern void set_value_range_to_nonnull (value_range *, tree);