[PATCH 4/4] Data structure is used for inline_summary struct.

[Martin Liška]

On 11/13/2014 05:04 PM, Jan Hubicka wrote:
>> +  if (!inline_summary_summary)
>> +    inline_summary_summary = (inline_summary_cgraph_summary *) inline_summary_cgraph_summary::create_ggc (symtab);
>
> Hehe, this is funny naming scheme.
> Peraps inline_summary_d and inline_summary_t for the data and type?

Hello.

I adopted suggested naming scheme.

>> -
>> -static void
>> -inline_node_duplication_hook (struct cgraph_node *src,
>> -			      struct cgraph_node *dst,
>> -			      ATTRIBUTE_UNUSED void *data)
>> +void
>> +inline_summary_cgraph_summary::duplication_hook (cgraph_node *src,
>> +			      cgraph_node *dst,
>> +			      inline_summary *,
>> +			      inline_summary *info)
>
> Becuase those are no longer "hooks" but virtual function, I guess we could call them
> simply duplicate/insert/remove.

Agree with the change.

>
> In a way I would like to see these to be methods of the underlying type rather than
> virtual methods of the summary, becuase these are operations on the data themselves.
> I was thinking to model these by specual constructor and copy constructor
> (taking the extra node pointer parameters) and standard destructor.  I am not sure this
> would be more understandable this way?

Motivation for this implementation is:
a) it's useful to have an access to cgraph_node that is associated with a sumary
b) with GTY, we cannot call destructors

>> -/* Need a typedef for inline_summary because of inline function
>> -   'inline_summary' below.  */
>> -typedef struct inline_summary inline_summary_t;
>> -extern GTY(()) vec<inline_summary_t, va_gc> *inline_summary_vec;
>> +class GTY((user)) inline_summary_cgraph_summary: public cgraph_summary <inline_summary *>
>> +{
>> +public:
>> +  inline_summary_cgraph_summary (symbol_table *symtab, bool ggc):
>> +    cgraph_summary <inline_summary *> (symtab, ggc) {}
>> +
>> +  static inline_summary_cgraph_summary *create_ggc (symbol_table *symtab)
>> +  {
>> +    inline_summary_cgraph_summary *summary = new (ggc_cleared_alloc <inline_summary_cgraph_summary> ()) inline_summary_cgraph_summary(symtab, true);
>> +    summary->disable_insertion_hook ();
>> +    return summary;
>> +  }
>> +
>> +
>> +  virtual void insertion_hook (cgraph_node *, inline_summary *);
>> +  virtual void removal_hook (cgraph_node *node, inline_summary *);
>> +  virtual void duplication_hook (cgraph_node *src, cgraph_node *dst, inline_summary *src_data, inline_summary *dst_data);
>> +};
>> +
>> +extern GTY(()) cgraph_summary <inline_summary *> *inline_summary_summary;
>
> All in all it looks better than original code.  If we moved insert/
>>
>>   /* Information kept about parameter of call site.  */
>>   struct inline_param_summary
>> @@ -249,10 +265,10 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool, bool, int *,
>>   extern int ncalls_inlined;
>>   extern int nfunctions_inlined;
>>
>> -static inline struct inline_summary *
>> -inline_summary (struct cgraph_node *node)
>> +static inline inline_summary *
>> +get_inline_summary (const struct cgraph_node *node)
>>   {
>> -  return &(*inline_summary_vec)[node->uid];
>> +  return (*inline_summary_summary)[node->summary_uid];
>
> Hmm, i guess there is no way to avoid the (*...)? Otherwise it would be cleaner
> to use inline_summary[...] instead of get_inline_summary IMO.

I added function_summary::get method, where the usage looks cleaner:
inline_summary_d->get (node).

Thanks,
Martin
  
> Thanks for working on this!
> Honza
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Data-structure-is-used-for-inline_summary-struct.patch
Type: text/x-patch
Size: 46104 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20141114/f9f52249/attachment.bin>