Provide an API for ipa_vr.

Message ID 20230517141622.464538-1-aldyh@redhat.com
State New
Headers
Series Provide an API for ipa_vr. |

Commit Message

Aldy Hernandez May 17, 2023, 2:16 p.m. UTC
  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(-)
  

Comments

Aldy Hernandez May 22, 2023, 6:42 p.m. UTC | #1
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 <aldyh@redhat.com> 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_cst_ref_desc> 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
>
  
Martin Jambor May 24, 2023, 2:10 p.m. UTC | #2
Hello,

On Wed, May 17 2023, 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?

Yes, that is true and keeping the type around in ipa_vr is probably
easier than postponing the deallocation of parameter descriptors
somehow.

>
> OK for trunk?

Yes, thanks.

Martin

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

Patch

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_cst_ref_desc> 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" } } */