Re: [PATCH 4/4] Data structure is used for inline_summary struct.
2014-12-08 Martin Liska mli...@suse.cz * lto-partition.c (add_symbol_to_partition_1): New inline_summary_d is used. (undo_partition): Likewise. (lto_balanced_map): Likewise. gcc/ChangeLog: 2014-12-08 Martin Liska mli...@suse.cz * cgraphunit.c (symbol_table::process_new_functions): New inline_summary_d is used. * ipa-cp.c (ipcp_cloning_candidate_p): Likewise. (devirtualization_time_bonus): Likewise. (estimate_local_effects): Likewise. (ipcp_propagate_stage): Likewise. * ipa-inline-analysis.c (evaluate_conditions_for_known_args): Likewise. (evaluate_properties_for_edge): Likewise. (inline_summary_alloc): Likewise. (reset_inline_summary): New inline_summary argument is introduced. (inline_summary_t::remove): New function. (inline_summary_t::duplicate): Likewise. (dump_inline_edge_summary): New inline_summary_d is used. (dump_inline_summary): Likewise. (estimate_function_body_sizes): Likewise. (compute_inline_parameters): Likewise. (estimate_edge_devirt_benefit): Likewise. (estimate_node_size_and_time): Likewise. (inline_update_callee_summaries): Likewise. (inline_merge_summary): Likewise. (inline_update_overall_summary): Likewise. (simple_edge_hints): Likewise. (do_estimate_edge_time): Likewise. (estimate_time_after_inlining): Likewise. (estimate_size_after_inlining): Likewise. (do_estimate_growth): Likewise. (growth_likely_positive): Likewise. (inline_generate_summary): Likewise. (inline_read_section): Likewise. (inline_read_summary): Likewise. (inline_write_summary): Likewise. (inline_free_summary): Likewise. * ipa-inline-transform.c (clone_inlined_nodes): Likewise. (inline_call): Likewise. * ipa-inline.c (caller_growth_limits): Likewise. (can_inline_edge_p): Likewise. (want_early_inline_function_p): Likewise. (compute_uninlined_call_time): Likewise. (compute_inlined_call_time): Likewise. (big_speedup_p): Likewise. (want_inline_small_function_p): Likewise. (edge_badness): Likewise. (update_caller_keys): Likewise. (update_callee_keys): Likewise. (recursive_inlining): Likewise. (inline_small_functions): Likewise. (inline_to_all_callers): Likewise. (dump_overall_stats): Likewise. (early_inline_small_functions): Likewise. * ipa-inline.h: New class inline_summary_t replaces vecinline_summary_t. * ipa-split.c (execute_split_functions): New inline_summary_d is used. * ipa.c (walk_polymorphic_call_targets): Likewise. * tree-sra.c (ipa_sra_preliminary_function_checks): Likewise. Patch is OK. I only changed mind about inline_summary_d. It does not look good used with inline_summary_d-get() especially because _d prefix was used in C++ication days for type names only. Probably inline_summaries-get() is better. OK with this change. Thanks, Honza
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On 11/14/2014 04:24 PM, Martin Liška wrote:
On 11/14/2014 03:09 PM, Martin Liška wrote:
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(()) vecinline_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
Patch v3.
Martin
Version v4.
Martin
From 4f87fdf746532a9ad89fcf85246cc449a49ba09d Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Mon, 8 Dec 2014 11:33:08 +0100
Subject: [PATCH 3/3] symbol_summary is used for inline_summary.
gcc/lto/ChangeLog:
2014-12-08 Martin Liska mli...@suse.cz
* lto-partition.c (add_symbol_to_partition_1): New inline_summary_d
is used.
(undo_partition): Likewise.
(lto_balanced_map): Likewise.
gcc/ChangeLog:
2014-12-08 Martin Liska mli...@suse.cz
* cgraphunit.c (symbol_table::process_new_functions): New inline_summary_d
is used.
* ipa-cp.c (ipcp_cloning_candidate_p): Likewise.
(devirtualization_time_bonus): Likewise.
(estimate_local_effects): Likewise.
(ipcp_propagate_stage): Likewise.
* ipa-inline-analysis.c (evaluate_conditions_for_known_args): Likewise.
(evaluate_properties_for_edge): Likewise.
(inline_summary_alloc): Likewise.
(reset_inline_summary): New inline_summary argument is introduced.
(inline_summary_t::remove): New function.
(inline_summary_t::duplicate): Likewise.
(dump_inline_edge_summary): New inline_summary_d is used.
(dump_inline_summary): Likewise.
(estimate_function_body_sizes): Likewise.
(compute_inline_parameters): Likewise.
(estimate_edge_devirt_benefit): Likewise.
(estimate_node_size_and_time): Likewise.
(inline_update_callee_summaries): Likewise.
(inline_merge_summary): Likewise.
(inline_update_overall_summary): Likewise.
(simple_edge_hints): Likewise.
(do_estimate_edge_time): Likewise.
(estimate_time_after_inlining): Likewise.
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On 11/18/2014 11:25 PM, Martin Jambor wrote:
On Tue, Nov 18, 2014 at 07:59:26PM +0100, Jan Hubicka wrote:
Hi,
On Tue, Nov 18, 2014 at 04:39:00PM +0100, Jan Hubicka wrote:
On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote:
b) with GTY, we cannot call destructor
Everything in symbol table is expecitely memory managed (i.e. enver left
to be freed by garbage collector). It resists in GTY only to allow linking
garbage collected object from them and to get PCH working.
Well, if I understand the intent correctly, summaries are for stuff
that is not in the symbol table. For example jump functions are a
Correct.
vector of structures possibly containing trees, so everything has to
be in garbage collected memory.
When an edge is removed, it is necessary to be notified about it
immediately, for example to decrement rdesc_refcount (you might argue
that that should be done in a separate hook and not from within a
summary class but then you start to rely on hook invocation ordering
so I think it is better to eventually use the summaries for it too).
I do not see why ctors/dtors can not do the reference counting. In fact
this is how refcounting is done usually anyway?
Well, when there is no garbage collection involved then yes, that is
how you normally do it but in the GC case, there is the question of
what is the appropriate time to call destructor on garbage collected
data (like jump functions)?
I still fail to see problem here. Summaries are explicitly managed- they are
constructed at summary construction time or when new callgarph node is
introduced/duplicated. They are destroyed when callgarph node is destroyed or
whole summary is ddestroyed. It is job of the summary datastructure to call
proper ctors/dtors, not job of garbage collector that provides the underlying
memory management.
I do not think that all summaries (in the meaning of a description of
one particular symbol table node or call graph edge) are explicitely
managed. For example ipa_edge_args or ipa_agg_replacement_value
(which my alignment patch changes to ipcp_transformation_summary) are
allocated in GC memory because they contain trees.
If you have datastructure that points to something that is not
explicitly managed (i.e. tree expression), you just can not have
non-trivial constructor on that datastructure, because that is freed
transparently by gty that don't do destruction...
I admit to not being particularly bright today but that seems to be
exactly my point.
Well, in your case you have datastructure jump_function that contain a pointer
to tree (EXPR). What I am trying to explain is that I see no reson why
jump_function needs to be POD.
I never said that the summary object needs to be a POD, I only said I
liked the possibility of storing very simple objects (without wrapping
them in classes with constructors and destructors). That is of course
nothing more than my personal preference.
The tree pointed to by EXPR pointer can not
have a dtor by itself because GGC will not call it upon freeing.
It is true that jump_function lives in GGC memory (to make pointer to expr
work) but it never gets removed by ggc_collect because it is always pointed to
by the summary datastructure. There are two ways to free the jump_function
datastructure.
1) removing the symbol node it is attached to.
Here the symtab code will call removal hook that was registered by
container
template. The container will call destructor of jump_function and the
ggc_free
its memory
2) removing the summary. In this case I would again expect the container
template to walk all summaries and free them.
So even if your structure lives in GGC memory it is not really garbage
collected and thus the lack of machinery to call dtors at a time ggc decides to
free something is not a problem?
In fact looking at struct default_hashmap_traits, I see:
/* Called to dispose of the key and value before marking the entry as
deleted. */
templatetypename T static void remove (T v) { v.~T (); }
Now I see, I should have read your previous email more carefully, by
explicitely managed you mean that destructors will be called
explicitely by the summary infrastructure. I was wondering how you
wanted to rip the summaries out of GGC memory.
Well, I suppose that would work, and since explicit calls to
destructors are basically the counterpart of placement new that we
already plan to use, it might be actually be the proper C++ thing to
do.
(I am not sure I like it though, for all other purposes the summary
objects will look like managed by the garbage collector and only we
who read this thread will know that the lifetime of the object would
be decoupled from the allocation-span of its memory).
Thanks for the clarification,
Martin
Hello.
I tried to come up with ctor/dtor solution for types passes to symbol_summary
template class.
Example:
struct inline_summary
{
inline_summary (cgraph_node *node);
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On 11/14/2014 05:06 PM, Jan Hubicka wrote:
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
Yep, one would have node addition
ctor (symtab_node *); (or cgraph/varpool nodes for cgraph/varpool annotations)
that would default to ctor for implementations that do not care about node.
And node duplication ctor
ctor (summary , symtab_node *, symtab_node *)
that would default to copy constructor for data that do not need to be copied.
Hello.
I have no problem with such construction and destruction, we can also provide
base implementation.
I would say that main advantage (in addition to have a way to provide resonable
defaults) is to make ctors/dtors of the embedded classes working well, so one
can
for example embedd pointer_map and not care about its construction/destruction.
b) with GTY, we cannot call destructor
Everything in symbol table is expecitely memory managed (i.e. enver left
to be freed by garbage collector). It resists in GTY only to allow linking
garbage collected object from them and to get PCH working.
However GTY types need to be allocated by ggc_alloc and one can't call dtor.
This was main motivation for providing hooks instead of ctor/dtor API.
Maybe I miss something?
Thanks,
Martin
This is however quite cosmetic issue I would preffer our C++ guys to comment
on. We can
tweak this incrementally.
+void
+inline_summary_t::duplicate (cgraph_node *src,
+cgraph_node *dst,
+inline_summary *,
+inline_summary *info)
Also we should have a way to say that the annotation do not need to be
duplicated (for example
when we do not want to annotate inline clones). Probably by adding duplicate_p
predicate that
is called before the actual duplication happens?
The updated patch is OK, I will take a look on the main patch.
Honza
{
- struct inline_summary *info;
inline_summary_alloc ();
- info = inline_summary (dst);
- memcpy (info, inline_summary (src), sizeof (struct inline_summary));
+ memcpy (info, inline_summary_d-get (src), sizeof (inline_summary));
/* TODO: as an optimization, we may avoid copying conditions
that are known to be false or true. */
info-conds = vec_safe_copy (info-conds);
@@ -1328,7 +1309,7 @@ free_growth_caches (void)
static void
dump_inline_edge_summary (FILE *f, int indent, struct cgraph_node *node,
- struct inline_summary *info)
+ inline_summary *info)
{
struct cgraph_edge *edge;
for (edge = node-callees; edge; edge = edge-next_callee)
@@ -1345,8 +1326,8 @@ dump_inline_edge_summary (FILE *f, int indent, struct
cgraph_node *node,
? inlined : cgraph_inline_failed_string (edge- inline_failed),
indent, , es-loop_depth, edge-frequency,
es-call_stmt_size, es-call_stmt_time,
- (int) inline_summary (callee)-size / INLINE_SIZE_SCALE,
- (int) inline_summary (callee)-estimated_stack_size);
+ (int) inline_summary_d-get (callee)-size / INLINE_SIZE_SCALE,
+ (int) inline_summary_d-get (callee)-estimated_stack_size);
if (es-predicate)
{
@@ -1372,9 +1353,9 @@ dump_inline_edge_summary (FILE *f, int indent, struct
cgraph_node *node,
fprintf (f, %*sStack frame offset %i, callee self size %i,
callee size %i\n,
indent + 2, ,
- (int) inline_summary (callee)-stack_frame_offset,
- (int) inline_summary (callee)-estimated_self_stack_size,
- (int) inline_summary (callee)-estimated_stack_size);
+ (int) inline_summary_d-get (callee)-stack_frame_offset,
+ (int) inline_summary_d-get
(callee)-estimated_self_stack_size,
+ (int) inline_summary_d-get (callee)-estimated_stack_size);
dump_inline_edge_summary (f, indent + 2, callee, info);
}
}
@@ -1402,7 +1383,7 @@ dump_inline_summary (FILE *f, struct cgraph_node *node)
{
if (node-definition)
{
- struct inline_summary *s = inline_summary (node);
+ inline_summary *s = inline_summary_d-get (node);
size_time_entry *e;
int i;
fprintf (f, Inline summary for %s/%i, node-name (),
@@ -1725,7 +1706,7 @@ eliminated_by_inlining_prob (gimple stmt)
static void
set_cond_stmt_execution_predicate (struct ipa_node_params *info,
- struct inline_summary *summary,
+
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote: b) with GTY, we cannot call destructor Everything in symbol table is expecitely memory managed (i.e. enver left to be freed by garbage collector). It resists in GTY only to allow linking garbage collected object from them and to get PCH working. Well, if I understand the intent correctly, summaries are for stuff that is not in the symbol table. For example jump functions are a Correct. vector of structures possibly containing trees, so everything has to be in garbage collected memory. When an edge is removed, it is necessary to be notified about it immediately, for example to decrement rdesc_refcount (you might argue that that should be done in a separate hook and not from within a summary class but then you start to rely on hook invocation ordering so I think it is better to eventually use the summaries for it too). I do not see why ctors/dtors can not do the reference counting. In fact this is how refcounting is done usually anyway? Well, when there is no garbage collection involved then yes, that is how you normally do it but in the GC case, there is the question of what is the appropriate time to call destructor on garbage collected data (like jump functions)? Martin
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote: b) with GTY, we cannot call destructor Everything in symbol table is expecitely memory managed (i.e. enver left to be freed by garbage collector). It resists in GTY only to allow linking garbage collected object from them and to get PCH working. Well, if I understand the intent correctly, summaries are for stuff that is not in the symbol table. For example jump functions are a Correct. vector of structures possibly containing trees, so everything has to be in garbage collected memory. When an edge is removed, it is necessary to be notified about it immediately, for example to decrement rdesc_refcount (you might argue that that should be done in a separate hook and not from within a summary class but then you start to rely on hook invocation ordering so I think it is better to eventually use the summaries for it too). I do not see why ctors/dtors can not do the reference counting. In fact this is how refcounting is done usually anyway? Well, when there is no garbage collection involved then yes, that is how you normally do it but in the GC case, there is the question of what is the appropriate time to call destructor on garbage collected data (like jump functions)? I still fail to see problem here. Summaries are explicitly managed- they are constructed at summary construction time or when new callgarph node is introduced/duplicated. They are destroyed when callgarph node is destroyed or whole summary is ddestroyed. It is job of the summary datastructure to call proper ctors/dtors, not job of garbage collector that provides the underlying memory management. If you have datastructure that points to something that is not explicitly managed (i.e. tree expression), you just can not have non-trivial constructor on that datastructure, because that is freed transparently by gty that don't do destruction... Honza
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
Hi, On Tue, Nov 18, 2014 at 04:39:00PM +0100, Jan Hubicka wrote: On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote: b) with GTY, we cannot call destructor Everything in symbol table is expecitely memory managed (i.e. enver left to be freed by garbage collector). It resists in GTY only to allow linking garbage collected object from them and to get PCH working. Well, if I understand the intent correctly, summaries are for stuff that is not in the symbol table. For example jump functions are a Correct. vector of structures possibly containing trees, so everything has to be in garbage collected memory. When an edge is removed, it is necessary to be notified about it immediately, for example to decrement rdesc_refcount (you might argue that that should be done in a separate hook and not from within a summary class but then you start to rely on hook invocation ordering so I think it is better to eventually use the summaries for it too). I do not see why ctors/dtors can not do the reference counting. In fact this is how refcounting is done usually anyway? Well, when there is no garbage collection involved then yes, that is how you normally do it but in the GC case, there is the question of what is the appropriate time to call destructor on garbage collected data (like jump functions)? I still fail to see problem here. Summaries are explicitly managed- they are constructed at summary construction time or when new callgarph node is introduced/duplicated. They are destroyed when callgarph node is destroyed or whole summary is ddestroyed. It is job of the summary datastructure to call proper ctors/dtors, not job of garbage collector that provides the underlying memory management. I do not think that all summaries (in the meaning of a description of one particular symbol table node or call graph edge) are explicitely managed. For example ipa_edge_args or ipa_agg_replacement_value (which my alignment patch changes to ipcp_transformation_summary) are allocated in GC memory because they contain trees. If you have datastructure that points to something that is not explicitly managed (i.e. tree expression), you just can not have non-trivial constructor on that datastructure, because that is freed transparently by gty that don't do destruction... I admit to not being particularly bright today but that seems to be exactly my point. Martin
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
Hi,
On Tue, Nov 18, 2014 at 04:39:00PM +0100, Jan Hubicka wrote:
On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote:
b) with GTY, we cannot call destructor
Everything in symbol table is expecitely memory managed (i.e. enver
left
to be freed by garbage collector). It resists in GTY only to allow
linking
garbage collected object from them and to get PCH working.
Well, if I understand the intent correctly, summaries are for stuff
that is not in the symbol table. For example jump functions are a
Correct.
vector of structures possibly containing trees, so everything has to
be in garbage collected memory.
When an edge is removed, it is necessary to be notified about it
immediately, for example to decrement rdesc_refcount (you might argue
that that should be done in a separate hook and not from within a
summary class but then you start to rely on hook invocation ordering
so I think it is better to eventually use the summaries for it too).
I do not see why ctors/dtors can not do the reference counting. In fact
this is how refcounting is done usually anyway?
Well, when there is no garbage collection involved then yes, that is
how you normally do it but in the GC case, there is the question of
what is the appropriate time to call destructor on garbage collected
data (like jump functions)?
I still fail to see problem here. Summaries are explicitly managed- they
are
constructed at summary construction time or when new callgarph node is
introduced/duplicated. They are destroyed when callgarph node is destroyed
or
whole summary is ddestroyed. It is job of the summary datastructure to call
proper ctors/dtors, not job of garbage collector that provides the
underlying
memory management.
I do not think that all summaries (in the meaning of a description of
one particular symbol table node or call graph edge) are explicitely
managed. For example ipa_edge_args or ipa_agg_replacement_value
(which my alignment patch changes to ipcp_transformation_summary) are
allocated in GC memory because they contain trees.
If you have datastructure that points to something that is not
explicitly managed (i.e. tree expression), you just can not have
non-trivial constructor on that datastructure, because that is freed
transparently by gty that don't do destruction...
I admit to not being particularly bright today but that seems to be
exactly my point.
Well, in your case you have datastructure jump_function that contain a pointer
to tree (EXPR). What I am trying to explain is that I see no reson why
jump_function needs to be POD. The tree pointed to by EXPR pointer can not
have a dtor by itself because GGC will not call it upon freeing.
It is true that jump_function lives in GGC memory (to make pointer to expr
work) but it never gets removed by ggc_collect because it is always pointed to
by the summary datastructure. There are two ways to free the jump_function
datastructure.
1) removing the symbol node it is attached to.
Here the symtab code will call removal hook that was registered by
container
template. The container will call destructor of jump_function and the
ggc_free
its memory
2) removing the summary. In this case I would again expect the container
template to walk all summaries and free them.
So even if your structure lives in GGC memory it is not really garbage
collected and thus the lack of machinery to call dtors at a time ggc decides to
free something is not a problem?
In fact looking at struct default_hashmap_traits, I see:
/* Called to dispose of the key and value before marking the entry as
deleted. */
templatetypename T static void remove (T v) { v.~T (); }
This trait gets called by the underlying hash table so if you have explicitly
managed hashmap (in GGC memory or not), things just work. Only catch is that
if you let your hashmap to be garbage collected, then your dtor is not called.
So probably the dtors are working same way with Martin's summaries?
I guess we can follow same scheme here, have summary_traits that default
to calling correspondin ctors/dtors.
Honza
Martin
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On Tue, Nov 18, 2014 at 07:59:26PM +0100, Jan Hubicka wrote:
Hi,
On Tue, Nov 18, 2014 at 04:39:00PM +0100, Jan Hubicka wrote:
On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote:
b) with GTY, we cannot call destructor
Everything in symbol table is expecitely memory managed (i.e.
enver left
to be freed by garbage collector). It resists in GTY only to
allow linking
garbage collected object from them and to get PCH working.
Well, if I understand the intent correctly, summaries are for stuff
that is not in the symbol table. For example jump functions are a
Correct.
vector of structures possibly containing trees, so everything has to
be in garbage collected memory.
When an edge is removed, it is necessary to be notified about it
immediately, for example to decrement rdesc_refcount (you might
argue
that that should be done in a separate hook and not from within a
summary class but then you start to rely on hook invocation ordering
so I think it is better to eventually use the summaries for it too).
I do not see why ctors/dtors can not do the reference counting. In
fact
this is how refcounting is done usually anyway?
Well, when there is no garbage collection involved then yes, that is
how you normally do it but in the GC case, there is the question of
what is the appropriate time to call destructor on garbage collected
data (like jump functions)?
I still fail to see problem here. Summaries are explicitly managed- they
are
constructed at summary construction time or when new callgarph node is
introduced/duplicated. They are destroyed when callgarph node is
destroyed or
whole summary is ddestroyed. It is job of the summary datastructure to
call
proper ctors/dtors, not job of garbage collector that provides the
underlying
memory management.
I do not think that all summaries (in the meaning of a description of
one particular symbol table node or call graph edge) are explicitely
managed. For example ipa_edge_args or ipa_agg_replacement_value
(which my alignment patch changes to ipcp_transformation_summary) are
allocated in GC memory because they contain trees.
If you have datastructure that points to something that is not
explicitly managed (i.e. tree expression), you just can not have
non-trivial constructor on that datastructure, because that is freed
transparently by gty that don't do destruction...
I admit to not being particularly bright today but that seems to be
exactly my point.
Well, in your case you have datastructure jump_function that contain a pointer
to tree (EXPR). What I am trying to explain is that I see no reson why
jump_function needs to be POD.
I never said that the summary object needs to be a POD, I only said I
liked the possibility of storing very simple objects (without wrapping
them in classes with constructors and destructors). That is of course
nothing more than my personal preference.
The tree pointed to by EXPR pointer can not
have a dtor by itself because GGC will not call it upon freeing.
It is true that jump_function lives in GGC memory (to make pointer to expr
work) but it never gets removed by ggc_collect because it is always pointed to
by the summary datastructure. There are two ways to free the jump_function
datastructure.
1) removing the symbol node it is attached to.
Here the symtab code will call removal hook that was registered by
container
template. The container will call destructor of jump_function and the
ggc_free
its memory
2) removing the summary. In this case I would again expect the container
template to walk all summaries and free them.
So even if your structure lives in GGC memory it is not really garbage
collected and thus the lack of machinery to call dtors at a time ggc decides
to
free something is not a problem?
In fact looking at struct default_hashmap_traits, I see:
/* Called to dispose of the key and value before marking the entry as
deleted. */
templatetypename T static void remove (T v) { v.~T (); }
Now I see, I should have read your previous email more carefully, by
explicitely managed you mean that destructors will be called
explicitely by the summary infrastructure. I was wondering how you
wanted to rip the summaries out of GGC memory.
Well, I suppose that would work, and since explicit calls to
destructors are basically the counterpart of placement new that we
already plan to use, it might be actually be the proper C++ thing to
do.
(I am not sure I like it though, for all other purposes the summary
objects will look like managed by the garbage collector and only we
who read this thread will know that the lifetime of the object would
be decoupled from the allocation-span of its
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On Tue, Nov 18, 2014 at 07:59:26PM +0100, Jan Hubicka wrote:
Hi,
On Tue, Nov 18, 2014 at 04:39:00PM +0100, Jan Hubicka wrote:
On Fri, Nov 14, 2014 at 08:59:10PM +0100, Jan Hubicka wrote:
b) with GTY, we cannot call destructor
Everything in symbol table is expecitely memory managed (i.e.
enver left
to be freed by garbage collector). It resists in GTY only to
allow linking
garbage collected object from them and to get PCH working.
Well, if I understand the intent correctly, summaries are for stuff
that is not in the symbol table. For example jump functions are a
Correct.
vector of structures possibly containing trees, so everything has to
be in garbage collected memory.
When an edge is removed, it is necessary to be notified about it
immediately, for example to decrement rdesc_refcount (you might
argue
that that should be done in a separate hook and not from within a
summary class but then you start to rely on hook invocation ordering
so I think it is better to eventually use the summaries for it too).
I do not see why ctors/dtors can not do the reference counting. In
fact
this is how refcounting is done usually anyway?
Well, when there is no garbage collection involved then yes, that is
how you normally do it but in the GC case, there is the question of
what is the appropriate time to call destructor on garbage collected
data (like jump functions)?
I still fail to see problem here. Summaries are explicitly managed- they
are
constructed at summary construction time or when new callgarph node is
introduced/duplicated. They are destroyed when callgarph node is
destroyed or
whole summary is ddestroyed. It is job of the summary datastructure to
call
proper ctors/dtors, not job of garbage collector that provides the
underlying
memory management.
I do not think that all summaries (in the meaning of a description of
one particular symbol table node or call graph edge) are explicitely
managed. For example ipa_edge_args or ipa_agg_replacement_value
(which my alignment patch changes to ipcp_transformation_summary) are
allocated in GC memory because they contain trees.
If you have datastructure that points to something that is not
explicitly managed (i.e. tree expression), you just can not have
non-trivial constructor on that datastructure, because that is freed
transparently by gty that don't do destruction...
I admit to not being particularly bright today but that seems to be
exactly my point.
Well, in your case you have datastructure jump_function that contain a pointer
to tree (EXPR). What I am trying to explain is that I see no reson why
jump_function needs to be POD. The tree pointed to by EXPR pointer can not
have a dtor by itself because GGC will not call it upon freeing.
ggc does call dtors when it is about to sweep an object. however if you
explicitly call ggc_free on an object with a dtor you need to call the
dtor manually I believe.
It is true that jump_function lives in GGC memory (to make pointer to expr
work) but it never gets removed by ggc_collect because it is always pointed to
by the summary datastructure. There are two ways to free the jump_function
datastructure.
assuming it doesn't need to go into pch, and I would really think it
never does there's no real reason it has to live on the heap, you could
iterate over all the summaries and mark all the trees they point at
manually.
1) removing the symbol node it is attached to.
Here the symtab code will call removal hook that was registered by
container
template. The container will call destructor of jump_function and the
ggc_free
its memory
2) removing the summary. In this case I would again expect the container
template to walk all summaries and free them.
So even if your structure lives in GGC memory it is not really garbage
collected and thus the lack of machinery to call dtors at a time ggc decides
to
free something is not a problem?
In fact looking at struct default_hashmap_traits, I see:
/* Called to dispose of the key and value before marking the entry as
deleted. */
templatetypename T static void remove (T v) { v.~T (); }
This trait gets called by the underlying hash table so if you have explicitly
managed hashmap (in GGC memory or not), things just work. Only catch is that
if you let your hashmap to be garbage collected, then your dtor is not called.
actually I think it should be, ggc tracks when objects with non trivial
dtors are allocated and sets up finalizers for them to run the dtor.
Trev
So probably the dtors are working same way with Martin's summaries?
I guess we can follow same scheme here, have summary_traits that default
to calling correspondin
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
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(()) vecinline_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
From 6e8531d8d3659524e337c7c1d96596952c3ff0e8 Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Fri, 14 Nov 2014 14:54:12 +0100
Subject: [PATCH 3/3] Data structure is used for inline_summary struct.
gcc/ChangeLog:
2014-11-12 Martin Liska mli...@suse.cz
* cgraphunit.c (symbol_table::process_new_functions):
inline_summary_vec is replaced with inline_summary_t.
* ipa-cp.c (ipcp_cloning_candidate_p): Usage of inline_summary_d::get.
(devirtualization_time_bonus): Likewise.
(estimate_local_effects): Likewise.
(ipcp_propagate_stage): Likewise.
* ipa-inline-analysis.c (evaluate_conditions_for_known_args): Likewise.
(evaluate_properties_for_edge): Likewise.
(inline_summary_alloc): Deletion of old hook holders.
(reset_inline_summary): inline_summary is added as argument.
(inline_summary_cgraph_summary::removal_hook): New function.
(inline_summary_cgraph_summary::duplication_hook): Likewise.
(dump_inline_edge_summary): Struct keyword removed.
(dump_inline_summary): Likewise.
(estimate_function_body_sizes): Usage of inline_summary_d::get.
(compute_inline_parameters): Likewise.
(estimate_edge_devirt_benefit): Struct keyword removed.
(estimate_node_size_and_time): Likewise.
(inline_update_callee_summaries): Likewise.
(inline_merge_summary): Usage of inline_summary_d::get.
(inline_update_overall_summary): Likewise.
(simple_edge_hints): Likewise.
(do_estimate_edge_time): Likewise.
(estimate_time_after_inlining): Likewise.
(estimate_size_after_inlining): Likewise.
(do_estimate_growth): Likewise.
(growth_likely_positive): Likewise.
(inline_generate_summary):
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On 11/14/2014 03:09 PM, Martin Liška wrote:
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(()) vecinline_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
Patch v3.
Martin
From 7f57a3a762fecea9a20e307f06e868a73da98000 Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Fri, 14 Nov 2014 14:54:12 +0100
Subject: [PATCH 3/3] Data structure is used for inline_summary struct.
gcc/ChangeLog:
2014-11-12 Martin Liska mli...@suse.cz
* cgraphunit.c (symbol_table::process_new_functions):
inline_summary_vec is replaced with inline_summary_t.
* ipa-cp.c (ipcp_cloning_candidate_p): Usage of inline_summary_t::get.
(devirtualization_time_bonus): Likewise.
(estimate_local_effects): Likewise.
(ipcp_propagate_stage): Likewise.
* ipa-inline-analysis.c (evaluate_conditions_for_known_args): Likewise.
(evaluate_properties_for_edge): Likewise.
(inline_summary_alloc): Deletion of old hook holders.
(reset_inline_summary): inline_summary is added as argument.
(inline_summary_cgraph_summary::removal_hook): New function.
(inline_summary_cgraph_summary::duplication_hook): Likewise.
(dump_inline_edge_summary): Struct keyword removed.
(dump_inline_summary): Likewise.
(estimate_function_body_sizes): Usage of inline_summary_t::get.
(compute_inline_parameters): Likewise.
(estimate_edge_devirt_benefit): Struct keyword removed.
(estimate_node_size_and_time): Likewise.
(inline_update_callee_summaries): Likewise.
(inline_merge_summary): Usage of inline_summary_t::get.
(inline_update_overall_summary): Likewise.
(simple_edge_hints): Likewise.
(do_estimate_edge_time): Likewise.
(estimate_time_after_inlining): Likewise.
(estimate_size_after_inlining): Likewise.
(do_estimate_growth): Likewise.
(growth_likely_positive): Likewise.
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
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
Yep, one would have node addition
ctor (symtab_node *); (or cgraph/varpool nodes for cgraph/varpool annotations)
that would default to ctor for implementations that do not care about node.
And node duplication ctor
ctor (summary , symtab_node *, symtab_node *)
that would default to copy constructor for data that do not need to be copied.
I would say that main advantage (in addition to have a way to provide resonable
defaults) is to make ctors/dtors of the embedded classes working well, so one
can
for example embedd pointer_map and not care about its construction/destruction.
b) with GTY, we cannot call destructor
Everything in symbol table is expecitely memory managed (i.e. enver left
to be freed by garbage collector). It resists in GTY only to allow linking
garbage collected object from them and to get PCH working.
This is however quite cosmetic issue I would preffer our C++ guys to comment
on. We can
tweak this incrementally.
+void
+inline_summary_t::duplicate (cgraph_node *src,
+ cgraph_node *dst,
+ inline_summary *,
+ inline_summary *info)
Also we should have a way to say that the annotation do not need to be
duplicated (for example
when we do not want to annotate inline clones). Probably by adding duplicate_p
predicate that
is called before the actual duplication happens?
The updated patch is OK, I will take a look on the main patch.
Honza
{
- struct inline_summary *info;
inline_summary_alloc ();
- info = inline_summary (dst);
- memcpy (info, inline_summary (src), sizeof (struct inline_summary));
+ memcpy (info, inline_summary_d-get (src), sizeof (inline_summary));
/* TODO: as an optimization, we may avoid copying conditions
that are known to be false or true. */
info-conds = vec_safe_copy (info-conds);
@@ -1328,7 +1309,7 @@ free_growth_caches (void)
static void
dump_inline_edge_summary (FILE *f, int indent, struct cgraph_node *node,
- struct inline_summary *info)
+ inline_summary *info)
{
struct cgraph_edge *edge;
for (edge = node-callees; edge; edge = edge-next_callee)
@@ -1345,8 +1326,8 @@ dump_inline_edge_summary (FILE *f, int indent, struct
cgraph_node *node,
? inlined : cgraph_inline_failed_string (edge- inline_failed),
indent, , es-loop_depth, edge-frequency,
es-call_stmt_size, es-call_stmt_time,
-(int) inline_summary (callee)-size / INLINE_SIZE_SCALE,
-(int) inline_summary (callee)-estimated_stack_size);
+(int) inline_summary_d-get (callee)-size / INLINE_SIZE_SCALE,
+(int) inline_summary_d-get (callee)-estimated_stack_size);
if (es-predicate)
{
@@ -1372,9 +1353,9 @@ dump_inline_edge_summary (FILE *f, int indent, struct
cgraph_node *node,
fprintf (f, %*sStack frame offset %i, callee self size %i,
callee size %i\n,
indent + 2, ,
-(int) inline_summary (callee)-stack_frame_offset,
-(int) inline_summary (callee)-estimated_self_stack_size,
-(int) inline_summary (callee)-estimated_stack_size);
+(int) inline_summary_d-get (callee)-stack_frame_offset,
+(int) inline_summary_d-get
(callee)-estimated_self_stack_size,
+(int) inline_summary_d-get (callee)-estimated_stack_size);
dump_inline_edge_summary (f, indent + 2, callee, info);
}
}
@@ -1402,7 +1383,7 @@ dump_inline_summary (FILE *f, struct cgraph_node *node)
{
if (node-definition)
{
- struct inline_summary *s = inline_summary (node);
+ inline_summary *s = inline_summary_d-get (node);
size_time_entry *e;
int i;
fprintf (f, Inline summary for %s/%i, node-name (),
@@ -1725,7 +1706,7 @@ eliminated_by_inlining_prob (gimple stmt)
static void
set_cond_stmt_execution_predicate (struct ipa_node_params *info,
-struct inline_summary *summary,
+inline_summary *summary,
basic_block bb)
{
gimple last;
@@ -1810,7 +1791,7 @@ set_cond_stmt_execution_predicate (struct
ipa_node_params *info,
static void
set_switch_stmt_execution_predicate (struct ipa_node_params *info,
- struct inline_summary
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
On Fri, Nov 14, 2014 at 05:06:44PM +0100, Jan Hubicka wrote: 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 Yep, one would have node addition ctor (symtab_node *); (or cgraph/varpool nodes for cgraph/varpool annotations) that would default to ctor for implementations that do not care about node. And node duplication ctor ctor (summary , symtab_node *, symtab_node *) that would default to copy constructor for data that do not need to be copied. I would say that main advantage (in addition to have a way to provide resonable defaults) is to make ctors/dtors of the embedded classes working well, so one can for example embedd pointer_map and not care about its construction/destruction. It was actually me who suggested virtual methods instead of constructors. My idea was to make the summaries very light-weight and usable even for simple types. For example, during IPA-CP propagation phase, I use an (edge) summary that is basically only a pointer to cgraph_edge. I'd even prefer that the default duplication hook would just do an assignment or a memcpy. b) with GTY, we cannot call destructor Everything in symbol table is expecitely memory managed (i.e. enver left to be freed by garbage collector). It resists in GTY only to allow linking garbage collected object from them and to get PCH working. Well, if I understand the intent correctly, summaries are for stuff that is not in the symbol table. For example jump functions are a vector of structures possibly containing trees, so everything has to be in garbage collected memory. When an edge is removed, it is necessary to be notified about it immediately, for example to decrement rdesc_refcount (you might argue that that should be done in a separate hook and not from within a summary class but then you start to rely on hook invocation ordering so I think it is better to eventually use the summaries for it too). Thanks, Martin
[PATCH 4/4] Data structure is used for inline_summary struct.
gcc/ChangeLog:
2014-11-12 Martin Liska mli...@suse.cz
* cgraphunit.c (symbol_table::process_new_functions):
inline_summary_vec is replaced with inline_summary_summary.
* ipa-cp.c (ipcp_cloning_candidate_p): Usage of get_inline_summary.
(devirtualization_time_bonus): Likewise.
(estimate_local_effects): Likewise.
(ipcp_propagate_stage): Likewise.
* ipa-inline-analysis.c (evaluate_conditions_for_known_args): Likewise.
(evaluate_properties_for_edge): Likewise.
(inline_summary_alloc): Deletion of old hook holders.
(reset_inline_summary): inline_summary is added as argument.
(inline_summary_cgraph_summary::removal_hook): New function.
(inline_summary_cgraph_summary::duplication_hook): Likewise.
(dump_inline_edge_summary): Struct keyword removed.
(dump_inline_summary): Likewise.
(estimate_function_body_sizes): Usage of get_inline_summary.
(compute_inline_parameters): Likewise.
(estimate_edge_devirt_benefit): Struct keyword removed.
(estimate_node_size_and_time): Likewise.
(inline_update_callee_summaries): Likewise.
(inline_merge_summary): Usage of get_inline_summary.
(inline_update_overall_summary): Likewise.
(simple_edge_hints): Likewise.
(do_estimate_edge_time): Likewise.
(estimate_time_after_inlining): Likewise.
(estimate_size_after_inlining): Likewise.
(do_estimate_growth): Likewise.
(growth_likely_positive): Likewise.
(inline_generate_summary): inline_summary_summary is registered.
(inline_read_section): Struct keyword removed.
(inline_read_summary): Likewise.
(inline_write_summary): Likewise.
(inline_free_summary): Removal of old hook holders.
* ipa-inline-transform.c (clone_inlined_nodes): Usage of
get_inline_summary.
(inline_call): Likewise.
* ipa-inline.c (caller_growth_limits): Struct keyword is removed.
(can_inline_edge_p): Usage of get_inline_summary.
(want_early_inline_function_p): Struct keyword removed.
(compute_uninlined_call_time): Usage of get_inline_summary.
(compute_inlined_call_time): Likewise.
(big_speedup_p): Likewise.
(want_inline_small_function_p): Likewise.
(edge_badness): Likewise.
(update_caller_keys): Likewise.
(update_callee_keys): Likewise.
(recursive_inlining): Likewise.
(inline_small_functions): Likewise.
(inline_to_all_callers): Likewise.
(dump_overall_stats): Likewise.
(early_inline_small_functions): Likewise.
* ipa-inline.h (get_inline_summary): New function.
* ipa-split.c (execute_split_functions): Usage of get_inline_summary.
* ipa.c (walk_polymorphic_call_targets): inline_summary_vec is replaced
with inline_summary_summary.
* tree-sra.c (ipa_sra_preliminary_function_checks): Usage of
get_inline_summary.
gcc/lto/ChangeLog:
2014-11-12 Martin Liska mli...@suse.cz
* lto-partition.c (add_symbol_to_partition_1): Usage of
get_inline_summary.
(undo_partition): Likewise.
(lto_balanced_map): Likewise.
---
gcc/cgraphunit.c | 2 +-
gcc/ipa-cp.c | 10 +--
gcc/ipa-inline-analysis.c | 193 -
gcc/ipa-inline-transform.c | 6 +-
gcc/ipa-inline.c | 61 +++---
gcc/ipa-inline.h | 30 +--
gcc/ipa-split.c| 2 +-
gcc/ipa.c | 2 +-
gcc/lto/lto-partition.c| 10 +--
gcc/tree-sra.c | 2 +-
10 files changed, 154 insertions(+), 164 deletions(-)
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index dbbdc44..de032c1 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -338,7 +338,7 @@ symbol_table::process_new_functions (void)
if (state == IPA_SSA
!gimple_in_ssa_p (DECL_STRUCT_FUNCTION (fndecl)))
g-get_passes ()-execute_early_local_passes ();
- else if (inline_summary_vec != NULL)
+ else if (inline_summary_summary != NULL)
compute_inline_parameters (node, true);
free_dominance_info (CDI_POST_DOMINATORS);
free_dominance_info (CDI_DOMINATORS);
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index da589af..f721e72 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -552,7 +552,7 @@ ipcp_cloning_candidate_p (struct cgraph_node *node)
init_caller_stats (stats);
node-call_for_symbol_thunks_and_aliases (gather_caller_stats, stats,
false);
- if (inline_summary (node)-self_size stats.n_calls)
+ if (get_inline_summary (node)-self_size stats.n_calls)
{
if (dump_file)
fprintf (dump_file, Considering %s for cloning; code might shrink.\n,
@@ -1718,7 +1718,7 @@ devirtualization_time_bonus (struct cgraph_node *node,
for (ie = node-indirect_calls; ie; ie = ie-next_callee)
{
Re: [PATCH 4/4] Data structure is used for inline_summary struct.
+ 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?
-
-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.
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?
-/* Need a typedef for inline_summary because of inline function
- 'inline_summary' below. */
-typedef struct inline_summary inline_summary_t;
-extern GTY(()) vecinline_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.
Thanks for working on this!
Honza
