[RFC,1/3] call_summary to keep info about cgraph_edges

Hello,

this patch is an actual implementation of the call_summary class (I
hope the name is a good analogy to the function_summary we have, I am
opened to other suggestions).  I have kept the implementation close to
the existing one of function-summary, there are I think only two
notable differences:

  - there is no call_graph hook for new edge creation, which is in
    fact quite natural, so there is no virtual function for that
    event, and

  - edge UIDs start with zero, and the inliner does use it to index a
    vector, so not only did I need to remove asserts that UIDs are
    non-zero, I also had to actually increment them before using them
    because apparently our hash-map does not like zero hashes.

Apart from that, I have introduced a predicate method exists to both
summaries, in order to figure out whether a summary for a given
node/edge already exists.  This is useful for example for dumping,
there is no need for dumping to allocate new stuff by querying.

Moreover, I have also spotted what I think is a bug in
function_summary::symtab_removal which does not call the destructor of
the item it holds when a function is removed and the item is
garbage-collecotr allocated.  Fixed in a simple hunk below.

I'll be grateful for any comments and/or suggestions,

Martin

2017-02-24  Martin Jambor  <mjambor@suse.cz>

	* symbol-summary.h (function_summary): New method exists.
	(function_summary::symtab_removal): Deallocate through release.
	(call_summary): New class.
	(gt_ggc_mx): New overload.
	(gt_pch_nx): Likewise.
	(gt_pch_nx): Likewise.
---
 gcc/symbol-summary.h | 214 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 210 insertions(+), 4 deletions(-)

Dne 2017-02-27 17:35, Martin Jambor napsal:
> Hello,
> 
> this patch is an actual implementation of the call_summary class (I
> hope the name is a good analogy to the function_summary we have, I am
> opened to other suggestions).  I have kept the implementation close to
> the existing one of function-summary, there are I think only two
> notable differences:
> 
>   - there is no call_graph hook for new edge creation, which is in
>     fact quite natural, so there is no virtual function for that
>     event, and
> 
>   - edge UIDs start with zero, and the inliner does use it to index a
>     vector, so not only did I need to remove asserts that UIDs are
>     non-zero, I also had to actually increment them before using them
>     because apparently our hash-map does not like zero hashes.
> 
> Apart from that, I have introduced a predicate method exists to both
> summaries, in order to figure out whether a summary for a given
> node/edge already exists.  This is useful for example for dumping,
> there is no need for dumping to allocate new stuff by querying.
> 
> Moreover, I have also spotted what I think is a bug in
> function_summary::symtab_removal which does not call the destructor of
> the item it holds when a function is removed and the item is
> garbage-collecotr allocated.  Fixed in a simple hunk below.
> 
> I'll be grateful for any comments and/or suggestions,
> 
> Martin
> 
> 2017-02-24  Martin Jambor  <mjambor@suse.cz>
> 
> 	* symbol-summary.h (function_summary): New method exists.
> 	(function_summary::symtab_removal): Deallocate through release.
> 	(call_summary): New class.
> 	(gt_ggc_mx): New overload.
> 	(gt_pch_nx): Likewise.
> 	(gt_pch_nx): Likewise.

I like this - the way summaries was managed in ipa-prop was always bit 
twisty.
path is OK for next stage1. I guess we may consider putting 
cgraph_indirect_call_info into a summary too
and probably break it into multiple summaries (for example polymorhic 
call info can probably go separately)

Honza
> ---
>  gcc/symbol-summary.h | 214 
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 210 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/symbol-summary.h b/gcc/symbol-summary.h
> index 3bcd14522c8..313082d7d5d 100644
> --- a/gcc/symbol-summary.h
> +++ b/gcc/symbol-summary.h
> @@ -126,6 +126,12 @@ public:
>      return m_map.elements ();
>    }
> 
> +  /* Return true if a summary for the given NODE already exists.  */
> +  bool exists (cgraph_node *node)
> +  {
> +    return m_map.get (node->summary_uid) != NULL;
> +  }
> +
>    /* Enable insertion hook invocation.  */
>    void enable_insertion_hook ()
>    {
> @@ -160,10 +166,7 @@ public:
>      if (v)
>        {
>  	summary->remove (node, *v);
> -
> -	if (!summary->m_ggc)
> -	  delete (*v);
> -
> +	summary->release (*v);
>  	summary->m_map.remove (summary_uid);
>        }
>    }
> @@ -251,4 +254,207 @@ gt_pch_nx(function_summary<T *>* const& summary,
> gt_pointer_operator op,
>    gt_pch_nx (&summary->m_map, op, cookie);
>  }
> 
> +/* An impossible class templated by non-pointers so, which makes sure 
> that only
> +   summaries gathering pointers can be created.  */
> +
> +template <class T>
> +class call_summary
> +{
> +private:
> +  call_summary();
> +};
> +
> +/* Class to store auxiliary information about call graph edges.  */
> +
> +template <class T>
> +class GTY((user)) call_summary <T *>
> +{
> +public:
> +  /* Default construction takes SYMTAB as an argument.  */
> +  call_summary (symbol_table *symtab, bool ggc = false): m_ggc (ggc),
> +    m_map (13, ggc), m_released (false), m_symtab (symtab)
> +  {
> +    m_symtab_removal_hook =
> +      symtab->add_edge_removal_hook
> +      (call_summary::symtab_removal, this);
> +    m_symtab_duplication_hook =
> +      symtab->add_edge_duplication_hook
> +      (call_summary::symtab_duplication, this);
> +  }
> +
> +  /* Destructor.  */
> +  virtual ~call_summary ()
> +  {
> +    release ();
> +  }
> +
> +  /* Destruction method that can be called for GGT purpose.  */
> +  void release ()
> +  {
> +    if (m_released)
> +      return;
> +
> +    m_symtab->remove_edge_removal_hook (m_symtab_removal_hook);
> +    m_symtab->remove_edge_duplication_hook 
> (m_symtab_duplication_hook);
> +
> +    /* Release all summaries.  */
> +    typedef typename hash_map <map_hash, T *>::iterator map_iterator;
> +    for (map_iterator it = m_map.begin (); it != m_map.end (); ++it)
> +      release ((*it).second);
> +
> +    m_released = true;
> +  }
> +
> +  /* Traverses all summarys with a function F called with
> +     ARG as argument.  */
> +  template<typename Arg, bool (*f)(const T &, Arg)>
> +  void traverse (Arg a) const
> +  {
> +    m_map.traverse <f> (a);
> +  }
> +
> +  /* Basic implementation of removal operation.  */
> +  virtual void remove (cgraph_edge *, T *) {}
> +
> +  /* Basic implementation of duplication operation.  */
> +  virtual void duplicate (cgraph_edge *, cgraph_edge *, T *, T *) {}
> +
> +  /* Allocates new data that are stored within map.  */
> +  T* allocate_new ()
> +  {
> +    /* Call gcc_internal_because we do not want to call finalizer for
> +       a type T.  We call dtor explicitly.  */
> +    return m_ggc ? new (ggc_internal_alloc (sizeof (T))) T () : new T 
> () ;
> +  }
> +
> +  /* Release an item that is stored within map.  */
> +  void release (T *item)
> +  {
> +    if (m_ggc)
> +      {
> +	item->~T ();
> +	ggc_free (item);
> +      }
> +    else
> +      delete item;
> +  }
> +
> +  /* Getter for summary callgraph edge pointer.  */
> +  T* get (cgraph_edge *edge)
> +  {
> +    return get (hashable_uid (edge));
> +  }
> +
> +  /* Return number of elements handled by data structure.  */
> +  size_t elements ()
> +  {
> +    return m_map.elements ();
> +  }
> +
> +  /* Return true if a summary for the given EDGE already exists.  */
> +  bool exists (cgraph_edge *edge)
> +  {
> +    return m_map.get (hashable_uid (edge)) != NULL;
> +  }
> +
> +  /* Symbol removal hook that is registered to symbol table.  */
> +  static void symtab_removal (cgraph_edge *edge, void *data)
> +  {
> +    call_summary *summary = (call_summary <T *> *) (data);
> +
> +    int h_uid = summary->hashable_uid (edge);
> +    T **v = summary->m_map.get (h_uid);
> +
> +    if (v)
> +      {
> +	summary->remove (edge, *v);
> +	summary->release (*v);
> +	summary->m_map.remove (h_uid);
> +      }
> +  }
> +
> +  /* Symbol duplication hook that is registered to symbol table.  */
> +  static void symtab_duplication (cgraph_edge *edge1, cgraph_edge 
> *edge2,
> +				  void *data)
> +  {
> +    call_summary *summary = (call_summary <T *> *) (data);
> +    T **v = summary->m_map.get (summary->hashable_uid (edge1));
> +
> +    if (v)
> +      {
> +	/* This load is necessary, because we insert a new value!  */
> +	T *data = *v;
> +	T *duplicate = summary->allocate_new ();
> +	summary->m_map.put (summary->hashable_uid (edge2), duplicate);
> +	summary->duplicate (edge1, edge2, data, duplicate);
> +      }
> +  }
> +
> +protected:
> +  /* Indication if we use ggc summary.  */
> +  bool m_ggc;
> +
> +private:
> +  typedef int_hash <int, 0, -1> map_hash;
> +
> +  /* Getter for summary callgraph ID.  */
> +  T* get (int uid)
> +  {
> +    bool existed;
> +    T **v = &m_map.get_or_insert (uid, &existed);
> +    if (!existed)
> +      *v = allocate_new ();
> +
> +    return *v;
> +  }
> +
> +  /* Get a hashable uid of EDGE.  */
> +  int hashable_uid (cgraph_edge *edge)
> +  {
> +    /* Edge uids start at zero which our hash_map does not like.  */
> +    return edge->uid + 1;
> +  }
> +
> +  /* Main summary store, where summary ID is used as key.  */
> +  hash_map <map_hash, T *> m_map;
> +  /* Internal summary removal hook pointer.  */
> +  cgraph_edge_hook_list *m_symtab_removal_hook;
> +  /* Internal summary duplication hook pointer.  */
> +  cgraph_2edge_hook_list *m_symtab_duplication_hook;
> +  /* Indicates if the summary is released.  */
> +  bool m_released;
> +  /* Symbol table the summary is registered to.  */
> +  symbol_table *m_symtab;
> +
> +  template <typename U> friend void gt_ggc_mx (call_summary <U *> * 
> const &);
> +  template <typename U> friend void gt_pch_nx (call_summary <U *> * 
> const &);
> +  template <typename U> friend void gt_pch_nx (call_summary <U *> * 
> const &,
> +      gt_pointer_operator, void *);
> +};
> +
> +template <typename T>
> +void
> +gt_ggc_mx(call_summary<T *>* const &summary)
> +{
> +  gcc_checking_assert (summary->m_ggc);
> +  gt_ggc_mx (&summary->m_map);
> +}
> +
> +template <typename T>
> +void
> +gt_pch_nx(call_summary<T *>* const &summary)
> +{
> +  gcc_checking_assert (summary->m_ggc);
> +  gt_pch_nx (&summary->m_map);
> +}
> +
> +template <typename T>
> +void
> +gt_pch_nx(call_summary<T *>* const& summary, gt_pointer_operator op,
> +	  void *cookie)
> +{
> +  gcc_checking_assert (summary->m_ggc);
> +  gt_pch_nx (&summary->m_map, op, cookie);
> +}
> +
>  #endif  /* GCC_SYMBOL_SUMMARY_H  */