[PATCH] Add value_range_base (w/o equivs)

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add value_range_base (w/o equivs)

Richard Biener

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 <[hidden email]>
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.

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);
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add value_range_base (w/o equivs)

Aldy Hernandez-2
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add value_range_base (w/o equivs)

Jeff Law
In reply to this post by Richard Biener
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 <[hidden email]>
> 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add value_range_base (w/o equivs)

Richard Biener
In reply to this post by Aldy Hernandez-2
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  <[hidden email]>

        * 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);
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add value_range_base (w/o equivs)

Aldy Hernandez-2


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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add value_range_base (w/o equivs)

Richard Biener
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.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add value_range_base (w/o equivs)

Aldy Hernandez-2
Ughhhh ok.

On Mon, Nov 12, 2018, 12:10 Richard Biener <[hidden email] 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.
>