I've adjusted the patch with some minor cleanups that came up when I implemented the rest of the IPA revamp. Retested. OK? On Wed, May 17, 2023 at 4:16 PM Aldy Hernandez wrote: > > This patch encapsulates the ipa_vr internals into an API. It also > makes it type agnostic, in preparation for upcoming changes to IPA. > > Interestingly, there's a 0.44% improvement to IPA-cp, which I'm sure > we'll soak up with future changes in this area :). > > BTW, there's a note here: > + // vrange_storage is typeless, but we need to know what type of > + // range that is being streamed out (irange, frange, etc). AFAICT, > + // there's no way to get at the underlying type by the time we > + // stream out in write_ipcp_transformation_info. > + tree m_type; > > Could someone more IPA savvy double check this is indeed the case? > > OK for trunk? > > gcc/ChangeLog: > > * ipa-cp.cc (ipa_value_range_from_jfunc): Use new ipa_vr API. > (ipcp_store_vr_results): Same. > * ipa-prop.cc (ipa_vr::ipa_vr): New. > (ipa_vr::get_vrange): New. > (ipa_vr::set_unknown): New. > (ipa_vr::streamer_read): New. > (ipa_vr::streamer_write): New. > (write_ipcp_transformation_info): Use new ipa_vr API. > (read_ipcp_transformation_info): Same. > (ipa_vr::nonzero_p): Delete. > (ipcp_update_vr): Use new ipa_vr API. > * ipa-prop.h (class ipa_vr): Provide an API and hide internals. > * ipa-sra.cc (zap_useless_ipcp_results): Use new ipa_vr API. > * gcc.dg/ipa/pr78121.c: Adjust for vrange::dump use. > * gcc.dg/ipa/vrp1.c: Same. > * gcc.dg/ipa/vrp2.c: Same. > * gcc.dg/ipa/vrp3.c: Same. > * gcc.dg/ipa/vrp4.c: Same. > * gcc.dg/ipa/vrp5.c: Same. > * gcc.dg/ipa/vrp6.c: Same. > * gcc.dg/ipa/vrp7.c: Same. > * gcc.dg/ipa/vrp8.c: Same. > --- > gcc/ipa-cp.cc | 22 ++--- > gcc/ipa-prop.cc | 129 ++++++++++++++++------------- > gcc/ipa-prop.h | 25 ++++-- > gcc/ipa-sra.cc | 4 +- > gcc/testsuite/gcc.dg/ipa/pr78121.c | 2 +- > gcc/testsuite/gcc.dg/ipa/vrp1.c | 4 +- > gcc/testsuite/gcc.dg/ipa/vrp2.c | 4 +- > gcc/testsuite/gcc.dg/ipa/vrp3.c | 2 +- > gcc/testsuite/gcc.dg/ipa/vrp4.c | 2 +- > gcc/testsuite/gcc.dg/ipa/vrp5.c | 2 +- > gcc/testsuite/gcc.dg/ipa/vrp6.c | 2 +- > gcc/testsuite/gcc.dg/ipa/vrp7.c | 2 +- > gcc/testsuite/gcc.dg/ipa/vrp8.c | 2 +- > 13 files changed, 109 insertions(+), 93 deletions(-) > > diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc > index 8cd0fa2cae7..d4b9d4ac27e 100644 > --- a/gcc/ipa-cp.cc > +++ b/gcc/ipa-cp.cc > @@ -1947,13 +1947,11 @@ ipa_value_range_from_jfunc (ipa_node_params *info, cgraph_edge *cs, > > idx = ipa_get_jf_pass_through_formal_id (jfunc); > > - if (!(*sum->m_vr)[idx].known) > + if (!(*sum->m_vr)[idx].known_p ()) > return vr; > tree vr_type = ipa_get_type (info, idx); > - value_range srcvr (vr_type, > - (*sum->m_vr)[idx].min, > - (*sum->m_vr)[idx].max, > - (*sum->m_vr)[idx].type); > + value_range srcvr; > + (*sum->m_vr)[idx].get_vrange (srcvr, vr_type); > > enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > > @@ -6621,25 +6619,19 @@ ipcp_store_vr_results (void) > for (unsigned i = 0; i < count; i++) > { > ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i); > - ipa_vr vr; > > if (!plats->m_value_range.bottom_p () > && !plats->m_value_range.top_p () > && dbg_cnt (ipa_cp_vr)) > { > - tree min, max; > - vr.known = true; > - vr.type = get_legacy_range (plats->m_value_range.m_vr, min, max); > - vr.min = wi::to_wide (min); > - vr.max = wi::to_wide (max); > + ipa_vr vr (plats->m_value_range.m_vr); > + ts->m_vr->quick_push (vr); > } > else > { > - vr.known = false; > - vr.type = VR_VARYING; > - vr.min = vr.max = wi::zero (INT_TYPE_SIZE); > + ipa_vr vr; > + ts->m_vr->quick_push (vr); > } > - ts->m_vr->quick_push (vr); > } > } > } > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index d7d70e5ec68..4ace410de49 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3. If not see > #include "symtab-clones.h" > #include "attr-fnspec.h" > #include "gimple-range.h" > +#include "value-range-storage.h" > > /* Function summary where the parameter infos are actually stored. */ > ipa_node_params_t *ipa_node_params_sum = NULL; > @@ -177,6 +178,66 @@ struct ipa_cst_ref_desc > static object_allocator ipa_refdesc_pool > ("IPA-PROP ref descriptions"); > > +ipa_vr::ipa_vr () > + : m_storage (NULL), > + m_type (NULL) > +{ > +} > + > +ipa_vr::ipa_vr (const vrange &r) > + : m_storage (ggc_alloc_vrange_storage (r)), > + m_type (r.type ()) > +{ > +} > + > +void > +ipa_vr::get_vrange (vrange &r, tree type) const > +{ > + m_storage->get_vrange (r, type); > +} > + > +void > +ipa_vr::set_unknown () > +{ > + if (m_storage) > + ggc_free (m_storage); > + > + m_storage = NULL; > +} > + > +void > +ipa_vr::streamer_read (lto_input_block *ib, data_in *data_in) > +{ > + struct bitpack_d bp = streamer_read_bitpack (ib); > + bool known = bp_unpack_value (&bp, 1); > + if (known) > + { > + Value_Range vr; > + streamer_read_value_range (ib, data_in, vr); > + if (!m_storage || !m_storage->fits_p (vr)) > + { > + if (m_storage) > + ggc_free (m_storage); > + m_storage = ggc_alloc_vrange_storage (vr); > + } > + m_storage->set_vrange (vr); > + } > +} > + > +void > +ipa_vr::streamer_write (output_block *ob) const > +{ > + struct bitpack_d bp = bitpack_create (ob->main_stream); > + bp_pack_value (&bp, !!m_storage, 1); > + streamer_write_bitpack (&bp); > + if (m_storage) > + { > + Value_Range vr (m_type); > + m_storage->get_vrange (vr, m_type); > + streamer_write_vrange (ob, vr); > + } > +} > + > /* Return true if DECL_FUNCTION_SPECIFIC_OPTIMIZATION of the decl associated > with NODE should prevent us from analyzing it for the purposes of IPA-CP. */ > > @@ -5338,19 +5399,7 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node, > > streamer_write_uhwi (ob, vec_safe_length (ts->m_vr)); > for (const ipa_vr &parm_vr : ts->m_vr) > - { > - struct bitpack_d bp; > - bp = bitpack_create (ob->main_stream); > - bp_pack_value (&bp, parm_vr.known, 1); > - streamer_write_bitpack (&bp); > - if (parm_vr.known) > - { > - streamer_write_enum (ob->main_stream, value_rang_type, > - VR_LAST, parm_vr.type); > - streamer_write_wide_int (ob, parm_vr.min); > - streamer_write_wide_int (ob, parm_vr.max); > - } > - } > + parm_vr.streamer_write (ob); > > streamer_write_uhwi (ob, vec_safe_length (ts->bits)); > for (const ipa_bits *bits_jfunc : ts->bits) > @@ -5401,16 +5450,7 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, > { > ipa_vr *parm_vr; > parm_vr = &(*ts->m_vr)[i]; > - struct bitpack_d bp; > - bp = streamer_read_bitpack (ib); > - parm_vr->known = bp_unpack_value (&bp, 1); > - if (parm_vr->known) > - { > - parm_vr->type = streamer_read_enum (ib, value_range_kind, > - VR_LAST); > - parm_vr->min = streamer_read_wide_int (ib); > - parm_vr->max = streamer_read_wide_int (ib); > - } > + parm_vr->streamer_read (ib, data_in); > } > } > count = streamer_read_uhwi (ib); > @@ -5848,19 +5888,6 @@ ipcp_update_bits (struct cgraph_node *node) > } > } > > -bool > -ipa_vr::nonzero_p (tree expr_type) const > -{ > - if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0)) > - return true; > - > - unsigned prec = TYPE_PRECISION (expr_type); > - return (type == VR_RANGE > - && TYPE_UNSIGNED (expr_type) > - && wi::eq_p (min, wi::one (prec)) > - && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type)))); > -} > - > /* Update value range of formal parameters as described in > ipcp_transformation. */ > > @@ -5909,38 +5936,22 @@ ipcp_update_vr (struct cgraph_node *node) > if (!ddef || !is_gimple_reg (parm)) > continue; > > - if (vr[i].known > - && (vr[i].type == VR_RANGE || vr[i].type == VR_ANTI_RANGE)) > + if (vr[i].known_p ()) > { > tree type = TREE_TYPE (ddef); > - unsigned prec = TYPE_PRECISION (type); > - if (INTEGRAL_TYPE_P (TREE_TYPE (ddef))) > + value_range tmp; > + vr[i].get_vrange (tmp, type); > + > + if (!tmp.undefined_p () && !tmp.varying_p ()) > { > if (dump_file) > { > fprintf (dump_file, "Setting value range of param %u " > "(now %i) ", i, remapped_idx); > - fprintf (dump_file, "%s[", > - (vr[i].type == VR_ANTI_RANGE) ? "~" : ""); > - print_decs (vr[i].min, dump_file); > - fprintf (dump_file, ", "); > - print_decs (vr[i].max, dump_file); > + tmp.dump (dump_file); > fprintf (dump_file, "]\n"); > } > - value_range v (type, > - wide_int_storage::from (vr[i].min, prec, > - TYPE_SIGN (type)), > - wide_int_storage::from (vr[i].max, prec, > - TYPE_SIGN (type)), > - vr[i].type); > - set_range_info (ddef, v); > - } > - else if (POINTER_TYPE_P (TREE_TYPE (ddef)) > - && vr[i].nonzero_p (TREE_TYPE (ddef))) > - { > - if (dump_file) > - fprintf (dump_file, "Setting nonnull for %u\n", i); > - set_ptr_nonnull (ddef); > + set_range_info (ddef, tmp); > } > } > } > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index d4936d4eaff..3b580ebb903 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -309,12 +309,25 @@ public: > class GTY(()) ipa_vr > { > public: > - /* The data fields below are valid only if known is true. */ > - bool known; > - enum value_range_kind type; > - wide_int min; > - wide_int max; > - bool nonzero_p (tree) const; > + ipa_vr (); > + ipa_vr (const vrange &); > + void set_unknown (); > + bool known_p () const { return m_storage != NULL; } > + void get_vrange (vrange &, tree type) const; > + void streamer_read (lto_input_block *, data_in *); > + void streamer_write (output_block *) const; > + > +private: > + friend void gt_pch_nx (struct ipa_vr &); > + friend void gt_ggc_mx (struct ipa_vr &); > + friend void gt_pch_nx (struct ipa_vr *, gt_pointer_operator, void *); > + > + vrange_storage *m_storage; > + // vrange_storage is typeless, but we need to know what type of > + // range that is being streamed out (irange, frange, etc). AFAICT, > + // there's no way to get at the underlying type by the time we > + // stream out in write_ipcp_transformation_info. > + tree m_type; > }; > > /* A jump function for a callsite represents the values passed as actual > diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc > index 7b8260bc9e1..09503cda748 100644 > --- a/gcc/ipa-sra.cc > +++ b/gcc/ipa-sra.cc > @@ -4081,11 +4081,11 @@ zap_useless_ipcp_results (const isra_func_summary *ifs, ipcp_transformation *ts) > bool useful_vr = false; > count = vec_safe_length (ts->m_vr); > for (unsigned i = 0; i < count; i++) > - if ((*ts->m_vr)[i].known) > + if ((*ts->m_vr)[i].known_p ()) > { > const isra_param_desc *desc = &(*ifs->m_parameters)[i]; > if (desc->locally_unused) > - (*ts->m_vr)[i].known = false; > + (*ts->m_vr)[i].set_unknown (); > else > useful_vr = true; > } > diff --git a/gcc/testsuite/gcc.dg/ipa/pr78121.c b/gcc/testsuite/gcc.dg/ipa/pr78121.c > index 19d6eda22f8..7e30834e645 100644 > --- a/gcc/testsuite/gcc.dg/ipa/pr78121.c > +++ b/gcc/testsuite/gcc.dg/ipa/pr78121.c > @@ -13,4 +13,4 @@ static void fn1(c) unsigned char c; > > void fn3() { fn1 (267); } > > -/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\) \\\[11, 35\\\]" "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\).* \\\[11, .*35\\\]" "cp" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp1.c b/gcc/testsuite/gcc.dg/ipa/vrp1.c > index e32a13c3d6a..42b8ec7785d 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp1.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp1.c > @@ -28,5 +28,5 @@ int main () > return 0; > } > > -/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\) \\\[6," "cp" } } */ > -/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\) \\\[0, 999\\\]" "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\).* \\\[6," "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\).* \\\[0, 999\\\]" "cp" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp2.c b/gcc/testsuite/gcc.dg/ipa/vrp2.c > index 31909bdbf24..b3ef9273891 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp2.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp2.c > @@ -31,5 +31,5 @@ int main () > return 0; > } > > -/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\) \\\[4," "cp" } } */ > -/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\) \\\[0, 11\\\]" "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\).* \\\[4," "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range of param 0 \\(now 0\\).* \\\[0, 11\\\]" "cp" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp3.c b/gcc/testsuite/gcc.dg/ipa/vrp3.c > index 9b1dcf98b25..171f0341e18 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp3.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp3.c > @@ -27,4 +27,4 @@ int main () > return 0; > } > > -/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\(now 0\\) \\\[0, 9\\\]" 2 "cp" } } */ > +/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\(now 0\\) .irange. int \\\[0, 9\\\]" 2 "cp" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp4.c b/gcc/testsuite/gcc.dg/ipa/vrp4.c > index 941f80e00b2..d02b09f2c84 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp4.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp4.c > @@ -24,5 +24,5 @@ int bar (struct st *s) > foo (&s->b); > } > > -/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range.* \\\[1, \\+INF\\\]" "cp" } } */ > /* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp5.c b/gcc/testsuite/gcc.dg/ipa/vrp5.c > index 571798dab51..6bbd3f16439 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp5.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp5.c > @@ -30,5 +30,5 @@ int bar (struct st *s) > foo (&arr2[1]); > } > > -/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range.* \\\[1, \\+INF\\\]" "cp" } } */ > /* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp6.c b/gcc/testsuite/gcc.dg/ipa/vrp6.c > index 971db443442..03e7ab93363 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp6.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp6.c > @@ -30,5 +30,5 @@ int bar (struct st *s) > foo (&b); > } > > -/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */ > +/* { dg-final { scan-ipa-dump "Setting value range.* \\\[1, \\+INF\\\]" "cp" } } */ > /* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp7.c b/gcc/testsuite/gcc.dg/ipa/vrp7.c > index ca5aa29e975..471c622a537 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp7.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp7.c > @@ -29,4 +29,4 @@ int main () > return 0; > } > > -/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\(now 0\\) \\\[-10, 9\\\]" 1 "cp" } } */ > +/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\(now 0\\) .irange. int \\\[-10, 9\\\]" 1 "cp" } } */ > diff --git a/gcc/testsuite/gcc.dg/ipa/vrp8.c b/gcc/testsuite/gcc.dg/ipa/vrp8.c > index 0ac5fb5277d..a01ffbcc757 100644 > --- a/gcc/testsuite/gcc.dg/ipa/vrp8.c > +++ b/gcc/testsuite/gcc.dg/ipa/vrp8.c > @@ -39,4 +39,4 @@ main () > return 0; > } > > -/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\(now 0\\) \\\[-10, 9\\\]" 1 "cp" } } */ > +/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\(now 0\\) .irange. int \\\[-10, 9\\\]" 1 "cp" } } */ > -- > 2.40.0 >