| Message ID | 20120831172412.GD6468@virgil.arch.suse.de |
|---|---|
| State | New |
| Headers | show |
On Fri, Aug 31, 2012 at 7:24 PM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Thu, Aug 30, 2012 at 05:11:35PM +0200, Martin Jambor wrote: >> this is a new version of the patch which makes ipa analysis produce >> predicates for PHI node results, at least at the bottom of the >> simplest diamond and semi-diamond CFG subgraphs. This time I also >> analyze the conditions again rather than extracting information from >> CFG edges, which means I can reason about substantially more PHI >> nodes. >> >> This patch makes us produce loop bounds hint for the pr48636.f90 >> testcase. >> >> Bootstrapped and tested on x86_64-linux. OK for trunk? >> >> Thanks, >> >> Martin >> >> >> 2012-08-29 Martin Jambor <mjambor@suse.cz> >> >> * ipa-inline-analysis.c (phi_result_unknown_predicate): New function. >> (predicate_for_phi_result): Likewise. >> (estimate_function_body_sizes): Use the above two functions. >> > > This patch, on top of the one doing loop calculations almost always, > introduces a number of testsuite failures which somehow I had not > caught during my testing. The problem is that either > calculate_dominance_info or loop_optimizer_init introduce new SSA > names for which there is no index in nonconstant_names which is > allocated before the dominance and loop computations. I'm currently > bootstrapping and testing the following fix which simply allocates the > vector after doing the two computations. If it passes I will commit > it straight away so that the regression is fixed before I leave for > the weekend, I hope it's obvious enough for that. > > On the other hand, it would really be better if we did not change > function bodies during IPA summary generation phase... Um ... we shouldn't do this. Can you track down where it happens? I suppose it might come from CFG manipulations loop_optimizer_init performs when not passing AVOID_CFG_MODIFICATIONS. Richard. > Sorry for the breakage, > > Martin > > > 2012-08-31 Martin Jambor <mjambor@suse.cz> > > * ipa-inline-analysis.c (estimate_function_body_sizes): Allocate > nonconstant_names after calculate_dominance_info and > loop_optimizer_init. > > Index: src/gcc/ipa-inline-analysis.c > =================================================================== > --- src.orig/gcc/ipa-inline-analysis.c > +++ src/gcc/ipa-inline-analysis.c > @@ -2185,13 +2185,6 @@ estimate_function_body_sizes (struct cgr > struct ipa_node_params *parms_info = NULL; > VEC (predicate_t, heap) *nonconstant_names = NULL; > > - if (ipa_node_params_vector && !early && optimize) > - { > - parms_info = IPA_NODE_REF (node); > - VEC_safe_grow_cleared (predicate_t, heap, nonconstant_names, > - VEC_length (tree, SSANAMES (my_function))); > - } > - > info->conds = 0; > info->entry = 0; > > @@ -2199,6 +2192,13 @@ estimate_function_body_sizes (struct cgr > { > calculate_dominance_info (CDI_DOMINATORS); > loop_optimizer_init (LOOPS_NORMAL | LOOPS_HAVE_RECORDED_EXITS); > + > + if (ipa_node_params_vector) > + { > + parms_info = IPA_NODE_REF (node); > + VEC_safe_grow_cleared (predicate_t, heap, nonconstant_names, > + VEC_length (tree, SSANAMES (my_function))); > + } > } > > if (dump_file) > > >
> On Fri, Aug 31, 2012 at 7:24 PM, Martin Jambor <mjambor@suse.cz> wrote: > > Hi, > > > > On Thu, Aug 30, 2012 at 05:11:35PM +0200, Martin Jambor wrote: > >> this is a new version of the patch which makes ipa analysis produce > >> predicates for PHI node results, at least at the bottom of the > >> simplest diamond and semi-diamond CFG subgraphs. This time I also > >> analyze the conditions again rather than extracting information from > >> CFG edges, which means I can reason about substantially more PHI > >> nodes. > >> > >> This patch makes us produce loop bounds hint for the pr48636.f90 > >> testcase. > >> > >> Bootstrapped and tested on x86_64-linux. OK for trunk? > >> > >> Thanks, > >> > >> Martin > >> > >> > >> 2012-08-29 Martin Jambor <mjambor@suse.cz> > >> > >> * ipa-inline-analysis.c (phi_result_unknown_predicate): New function. > >> (predicate_for_phi_result): Likewise. > >> (estimate_function_body_sizes): Use the above two functions. > >> > > > > This patch, on top of the one doing loop calculations almost always, > > introduces a number of testsuite failures which somehow I had not > > caught during my testing. The problem is that either > > calculate_dominance_info or loop_optimizer_init introduce new SSA > > names for which there is no index in nonconstant_names which is > > allocated before the dominance and loop computations. I'm currently > > bootstrapping and testing the following fix which simply allocates the > > vector after doing the two computations. If it passes I will commit > > it straight away so that the regression is fixed before I leave for > > the weekend, I hope it's obvious enough for that. > > > > On the other hand, it would really be better if we did not change > > function bodies during IPA summary generation phase... > > Um ... we shouldn't do this. Can you track down where it happens? I > suppose it might come from CFG manipulations loop_optimizer_init > performs when not passing AVOID_CFG_MODIFICATIONS. I bet it come from loop noromalization :) (i.e. loop closed form or preheader construction both needs new SSA names.) I think it would be best to make pass manager to handle this and make loop normalization to happen once before all SSA IPA analysis Honza
On Mon, Sep 3, 2012 at 5:52 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> On Fri, Aug 31, 2012 at 7:24 PM, Martin Jambor <mjambor@suse.cz> wrote: >> > Hi, >> > >> > On Thu, Aug 30, 2012 at 05:11:35PM +0200, Martin Jambor wrote: >> >> this is a new version of the patch which makes ipa analysis produce >> >> predicates for PHI node results, at least at the bottom of the >> >> simplest diamond and semi-diamond CFG subgraphs. This time I also >> >> analyze the conditions again rather than extracting information from >> >> CFG edges, which means I can reason about substantially more PHI >> >> nodes. >> >> >> >> This patch makes us produce loop bounds hint for the pr48636.f90 >> >> testcase. >> >> >> >> Bootstrapped and tested on x86_64-linux. OK for trunk? >> >> >> >> Thanks, >> >> >> >> Martin >> >> >> >> >> >> 2012-08-29 Martin Jambor <mjambor@suse.cz> >> >> >> >> * ipa-inline-analysis.c (phi_result_unknown_predicate): New function. >> >> (predicate_for_phi_result): Likewise. >> >> (estimate_function_body_sizes): Use the above two functions. >> >> >> > >> > This patch, on top of the one doing loop calculations almost always, >> > introduces a number of testsuite failures which somehow I had not >> > caught during my testing. The problem is that either >> > calculate_dominance_info or loop_optimizer_init introduce new SSA >> > names for which there is no index in nonconstant_names which is >> > allocated before the dominance and loop computations. I'm currently >> > bootstrapping and testing the following fix which simply allocates the >> > vector after doing the two computations. If it passes I will commit >> > it straight away so that the regression is fixed before I leave for >> > the weekend, I hope it's obvious enough for that. >> > >> > On the other hand, it would really be better if we did not change >> > function bodies during IPA summary generation phase... >> >> Um ... we shouldn't do this. Can you track down where it happens? I >> suppose it might come from CFG manipulations loop_optimizer_init >> performs when not passing AVOID_CFG_MODIFICATIONS. > > I bet it come from loop noromalization :) (i.e. loop closed form > or preheader construction both needs new SSA names.) > I think it would be best to make pass manager to handle this and make > loop normalization to happen once before all SSA IPA analysis And compute loops as well. Richard. > Honza >
On Tue, Sep 04, 2012 at 11:27:47AM +0200, Richard Guenther wrote: > On Mon, Sep 3, 2012 at 5:52 PM, Jan Hubicka <hubicka@ucw.cz> wrote: > >> On Fri, Aug 31, 2012 at 7:24 PM, Martin Jambor <mjambor@suse.cz> wrote: > >> > Hi, > >> > > >> > On Thu, Aug 30, 2012 at 05:11:35PM +0200, Martin Jambor wrote: > >> >> this is a new version of the patch which makes ipa analysis produce > >> >> predicates for PHI node results, at least at the bottom of the > >> >> simplest diamond and semi-diamond CFG subgraphs. This time I also > >> >> analyze the conditions again rather than extracting information from > >> >> CFG edges, which means I can reason about substantially more PHI > >> >> nodes. > >> >> > >> >> This patch makes us produce loop bounds hint for the pr48636.f90 > >> >> testcase. > >> >> > >> >> Bootstrapped and tested on x86_64-linux. OK for trunk? > >> >> > >> >> Thanks, > >> >> > >> >> Martin > >> >> > >> >> > >> >> 2012-08-29 Martin Jambor <mjambor@suse.cz> > >> >> > >> >> * ipa-inline-analysis.c (phi_result_unknown_predicate): New function. > >> >> (predicate_for_phi_result): Likewise. > >> >> (estimate_function_body_sizes): Use the above two functions. > >> >> > >> > > >> > This patch, on top of the one doing loop calculations almost always, > >> > introduces a number of testsuite failures which somehow I had not > >> > caught during my testing. The problem is that either > >> > calculate_dominance_info or loop_optimizer_init introduce new SSA > >> > names for which there is no index in nonconstant_names which is > >> > allocated before the dominance and loop computations. I'm currently > >> > bootstrapping and testing the following fix which simply allocates the > >> > vector after doing the two computations. If it passes I will commit > >> > it straight away so that the regression is fixed before I leave for > >> > the weekend, I hope it's obvious enough for that. > >> > > >> > On the other hand, it would really be better if we did not change > >> > function bodies during IPA summary generation phase... > >> > >> Um ... we shouldn't do this. Can you track down where it happens? I > >> suppose it might come from CFG manipulations loop_optimizer_init > >> performs when not passing AVOID_CFG_MODIFICATIONS. > > > > I bet it come from loop noromalization :) (i.e. loop closed form > > or preheader construction both needs new SSA names.) > > I think it would be best to make pass manager to handle this and make > > loop normalization to happen once before all SSA IPA analysis > > And compute loops as well. OK, this is now PR 54477 so that we don't forget. Thanks, Martin
Index: src/gcc/ipa-inline-analysis.c =================================================================== --- src.orig/gcc/ipa-inline-analysis.c +++ src/gcc/ipa-inline-analysis.c @@ -2185,13 +2185,6 @@ estimate_function_body_sizes (struct cgr struct ipa_node_params *parms_info = NULL; VEC (predicate_t, heap) *nonconstant_names = NULL; - if (ipa_node_params_vector && !early && optimize) - { - parms_info = IPA_NODE_REF (node); - VEC_safe_grow_cleared (predicate_t, heap, nonconstant_names, - VEC_length (tree, SSANAMES (my_function))); - } - info->conds = 0; info->entry = 0; @@ -2199,6 +2192,13 @@ estimate_function_body_sizes (struct cgr { calculate_dominance_info (CDI_DOMINATORS); loop_optimizer_init (LOOPS_NORMAL | LOOPS_HAVE_RECORDED_EXITS); + + if (ipa_node_params_vector) + { + parms_info = IPA_NODE_REF (node); + VEC_safe_grow_cleared (predicate_t, heap, nonconstant_names, + VEC_length (tree, SSANAMES (my_function))); + } } if (dump_file)