* Minor fixes to ipa-inline-analysis.c
@ 2012-11-07 9:25 Jan Hubicka
2012-11-25 11:26 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jan Hubicka @ 2012-11-07 9:25 UTC (permalink / raw)
To: gcc-patches
Hi,
while analyzing c-ray I noticed two issues. First is that I originally set number
of size/time entries to 32. Once we reach this limit we conservatively account
everything as unconditional. This limit is not met on relatively simple
testcases, like ray-sphere. The reason is that aggregate tracking now adds a lot
of new conditionals on individual fields. While number of arguments hardly exceeds
5, the number of fields passed and used easilly. So there is need to increase
the bound.
While propagating info about non-constant parameters, we should also work in
reverse postorder rather than in random order, since we can propagate things
down across SSA graph.
Bootstrapped/regtested & comitted.
Honza
Index: ChangeLog
===================================================================
--- ChangeLog (revision 193284)
+++ ChangeLog (working copy)
@@ -1,3 +1,12 @@
+2012-11-07 Jan Hubicka <jh@suse.cz>
+
+ * ipa-inline-analysis.c (true_predicate, single_cond_predicate,
+ reset_inline_edge_summary): Fix
+ formatting.
+ (account_size_time): Bump up the limit on number of size/time entries to
+ 256.
+ (estimate_function_body_sizes): Work in reverse postorder.
+
2012-11-07 David S. Miller <davem@davemloft.net>
PR bootstrap/55211
Index: ipa-inline-analysis.c
===================================================================
--- ipa-inline-analysis.c (revision 193284)
+++ ipa-inline-analysis.c (working copy)
@@ -149,7 +149,7 @@ static inline struct predicate
true_predicate (void)
{
struct predicate p;
- p.clause[0]=0;
+ p.clause[0] = 0;
return p;
}
@@ -160,8 +160,8 @@ static inline struct predicate
single_cond_predicate (int cond)
{
struct predicate p;
- p.clause[0]=1 << cond;
- p.clause[1]=0;
+ p.clause[0] = 1 << cond;
+ p.clause[1] = 0;
return p;
}
@@ -692,12 +692,14 @@ account_size_time (struct inline_summary
found = true;
break;
}
- if (i == 32)
+ if (i == 256)
{
i = 0;
found = true;
e = &VEC_index (size_time_entry, summary->entry, 0);
gcc_assert (!e->predicate.clause[0]);
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, "\t\tReached limit on number of entries, ignoring the predicate.");
}
if (dump_file && (dump_flags & TDF_DETAILS) && (time || size))
{
@@ -970,7 +972,7 @@ reset_inline_edge_summary (struct cgraph
{
struct inline_edge_summary *es = inline_edge_summary (e);
- es->call_stmt_size = es->call_stmt_time =0;
+ es->call_stmt_size = es->call_stmt_time = 0;
if (es->predicate)
pool_free (edge_predicate_pool, es->predicate);
es->predicate = NULL;
@@ -2280,6 +2282,8 @@ estimate_function_body_sizes (struct cgr
struct predicate bb_predicate;
struct ipa_node_params *parms_info = NULL;
VEC (predicate_t, heap) *nonconstant_names = NULL;
+ int nblocks, n;
+ int *order;
info->conds = 0;
info->entry = 0;
@@ -2312,8 +2316,12 @@ estimate_function_body_sizes (struct cgr
gcc_assert (my_function && my_function->cfg);
if (parms_info)
compute_bb_predicates (node, parms_info, info);
- FOR_EACH_BB_FN (bb, my_function)
+ gcc_assert (cfun == my_function);
+ order = XNEWVEC (int, n_basic_blocks);
+ nblocks = pre_and_rev_post_order_compute (NULL, order, false);
+ for (n = 0; n < nblocks; n++)
{
+ bb = BASIC_BLOCK (order[n]);
freq = compute_call_stmt_bb_frequency (node->symbol.decl, bb);
/* TODO: Obviously predicates can be propagated down across CFG. */
@@ -2486,6 +2494,7 @@ estimate_function_body_sizes (struct cgr
time = (time + CGRAPH_FREQ_BASE / 2) / CGRAPH_FREQ_BASE;
if (time > MAX_TIME)
time = MAX_TIME;
+ free (order);
if (!early && nonconstant_names)
{
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Minor fixes to ipa-inline-analysis.c
2012-11-07 9:25 Minor fixes to ipa-inline-analysis.c Jan Hubicka
@ 2012-11-25 11:26 ` Richard Biener
2012-11-26 9:52 ` Jan Hubicka
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2012-11-25 11:26 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches
On Wed, Nov 7, 2012 at 10:25 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> while analyzing c-ray I noticed two issues. First is that I originally set number
> of size/time entries to 32. Once we reach this limit we conservatively account
> everything as unconditional. This limit is not met on relatively simple
> testcases, like ray-sphere. The reason is that aggregate tracking now adds a lot
> of new conditionals on individual fields. While number of arguments hardly exceeds
> 5, the number of fields passed and used easilly. So there is need to increase
> the bound.
>
> While propagating info about non-constant parameters, we should also work in
> reverse postorder rather than in random order, since we can propagate things
> down across SSA graph.
>
> Bootstrapped/regtested & comitted.
>
> Honza
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 193284)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,12 @@
> +2012-11-07 Jan Hubicka <jh@suse.cz>
> +
> + * ipa-inline-analysis.c (true_predicate, single_cond_predicate,
> + reset_inline_edge_summary): Fix
> + formatting.
> + (account_size_time): Bump up the limit on number of size/time entries to
> + 256.
> + (estimate_function_body_sizes): Work in reverse postorder.
> +
> 2012-11-07 David S. Miller <davem@davemloft.net>
>
> PR bootstrap/55211
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c (revision 193284)
> +++ ipa-inline-analysis.c (working copy)
> @@ -149,7 +149,7 @@ static inline struct predicate
> true_predicate (void)
> {
> struct predicate p;
> - p.clause[0]=0;
> + p.clause[0] = 0;
> return p;
> }
>
> @@ -160,8 +160,8 @@ static inline struct predicate
> single_cond_predicate (int cond)
> {
> struct predicate p;
> - p.clause[0]=1 << cond;
> - p.clause[1]=0;
> + p.clause[0] = 1 << cond;
> + p.clause[1] = 0;
> return p;
> }
>
> @@ -692,12 +692,14 @@ account_size_time (struct inline_summary
> found = true;
> break;
> }
> - if (i == 32)
> + if (i == 256)
Shouldn't this be a --param?
> {
> i = 0;
> found = true;
> e = &VEC_index (size_time_entry, summary->entry, 0);
> gcc_assert (!e->predicate.clause[0]);
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file, "\t\tReached limit on number of entries, ignoring the predicate.");
> }
> if (dump_file && (dump_flags & TDF_DETAILS) && (time || size))
> {
> @@ -970,7 +972,7 @@ reset_inline_edge_summary (struct cgraph
> {
> struct inline_edge_summary *es = inline_edge_summary (e);
>
> - es->call_stmt_size = es->call_stmt_time =0;
> + es->call_stmt_size = es->call_stmt_time = 0;
> if (es->predicate)
> pool_free (edge_predicate_pool, es->predicate);
> es->predicate = NULL;
> @@ -2280,6 +2282,8 @@ estimate_function_body_sizes (struct cgr
> struct predicate bb_predicate;
> struct ipa_node_params *parms_info = NULL;
> VEC (predicate_t, heap) *nonconstant_names = NULL;
> + int nblocks, n;
> + int *order;
>
> info->conds = 0;
> info->entry = 0;
> @@ -2312,8 +2316,12 @@ estimate_function_body_sizes (struct cgr
> gcc_assert (my_function && my_function->cfg);
> if (parms_info)
> compute_bb_predicates (node, parms_info, info);
> - FOR_EACH_BB_FN (bb, my_function)
> + gcc_assert (cfun == my_function);
> + order = XNEWVEC (int, n_basic_blocks);
> + nblocks = pre_and_rev_post_order_compute (NULL, order, false);
> + for (n = 0; n < nblocks; n++)
> {
> + bb = BASIC_BLOCK (order[n]);
> freq = compute_call_stmt_bb_frequency (node->symbol.decl, bb);
>
> /* TODO: Obviously predicates can be propagated down across CFG. */
> @@ -2486,6 +2494,7 @@ estimate_function_body_sizes (struct cgr
> time = (time + CGRAPH_FREQ_BASE / 2) / CGRAPH_FREQ_BASE;
> if (time > MAX_TIME)
> time = MAX_TIME;
> + free (order);
>
> if (!early && nonconstant_names)
> {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Minor fixes to ipa-inline-analysis.c
2012-11-25 11:26 ` Richard Biener
@ 2012-11-26 9:52 ` Jan Hubicka
0 siblings, 0 replies; 3+ messages in thread
From: Jan Hubicka @ 2012-11-26 9:52 UTC (permalink / raw)
To: Richard Biener; +Cc: Jan Hubicka, gcc-patches
> > @@ -692,12 +692,14 @@ account_size_time (struct inline_summary
> > found = true;
> > break;
> > }
> > - if (i == 32)
> > + if (i == 256)
>
> Shouldn't this be a --param?
You are probably right. ipa-inline-analysis has few hard wired constants in it but mostly
to allow use bitmaps in int and constantly sized arrays for predicates. It was a plan that
those constants simply won't need tunning. In this case we can definitely allow user changes.
I will make a patch.
Honza
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-11-26 9:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-07 9:25 Minor fixes to ipa-inline-analysis.c Jan Hubicka
2012-11-25 11:26 ` Richard Biener
2012-11-26 9:52 ` Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).