| Message ID | alpine.LSU.2.20.1811091509460.1827@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Add value_range_base (w/o equivs) | expand |
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
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
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);
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
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.
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. >
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);
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.