| Message ID | 20120829182400.GC3395@virgil.arch.suse.de |
|---|---|
| State | New |
| Headers | show |
> Hi, > > the patch below fixes PR 54394. The problem is that since revision > 190346 we depend on bb->loop_father being non-NULL to get loop_depth. > However, with loops not computed, the loop_father is NULL, loop_depth > is thus considered zero and call graph edges out of such BB can be > considered much cooler, leading to inlining regressions. > > This patch fixes that by recomputing loops whenever optimizing, not > only for loop bounds hints. We might put the computation elsewhere or > do it only under more restrictive circumstances, but I believe that > after rev. 190346 we have to do it. In particular, I am not sure > whether we had (semi)correct loop_depths when doing early inlining or > not, this patch re-calculates it for early inliner too. > > Bootstrapped and tested on x86_64-linux, fixes fatigue run-time on > an x86_64-linux and i686-linux for me. What do you think? > > Thanks, > > Martin > > > 2012-08-29 Martin Jambor <mjambor@suse.cz> > > PR middle-end/54394 > * ipa-inline-analysis.c (estimate_function_body_sizes): Compute > dominance info and loops whenever optimizing. > > > Index: src/gcc/ipa-inline-analysis.c > =================================================================== > --- src.orig/gcc/ipa-inline-analysis.c > +++ src/gcc/ipa-inline-analysis.c > @@ -2102,6 +2102,11 @@ estimate_function_body_sizes (struct cgr > info->conds = 0; > info->entry = 0; > > + if (optimize) This is OK. I think you can also skip thi computation for early inlining where we don't do any hints, so probably if (optimize && !early) Honza
On Fri, Aug 31, 2012 at 10:53:32AM +0200, Jan Hubicka wrote: > > Hi, > > > > the patch below fixes PR 54394. The problem is that since revision > > 190346 we depend on bb->loop_father being non-NULL to get loop_depth. > > However, with loops not computed, the loop_father is NULL, loop_depth > > is thus considered zero and call graph edges out of such BB can be > > considered much cooler, leading to inlining regressions. > > > > This patch fixes that by recomputing loops whenever optimizing, not > > only for loop bounds hints. We might put the computation elsewhere or > > do it only under more restrictive circumstances, but I believe that > > after rev. 190346 we have to do it. In particular, I am not sure > > whether we had (semi)correct loop_depths when doing early inlining or > > not, this patch re-calculates it for early inliner too. > > > > Bootstrapped and tested on x86_64-linux, fixes fatigue run-time on > > an x86_64-linux and i686-linux for me. What do you think? > > > > Thanks, > > > > Martin > > > > > > 2012-08-29 Martin Jambor <mjambor@suse.cz> > > > > PR middle-end/54394 > > * ipa-inline-analysis.c (estimate_function_body_sizes): Compute > > dominance info and loops whenever optimizing. > > > > > > Index: src/gcc/ipa-inline-analysis.c > > =================================================================== > > --- src.orig/gcc/ipa-inline-analysis.c > > +++ src/gcc/ipa-inline-analysis.c > > @@ -2102,6 +2102,11 @@ estimate_function_body_sizes (struct cgr > > info->conds = 0; > > info->entry = 0; > > > > + if (optimize) > > This is OK. I think you can also skip thi computation for early inlining > where we don't do any hints, so probably if (optimize && !early) > > Honza This is not required to make hints working, it is necessary because of the following line a in estimate_function_body_sizes: es->loop_depth = bb_loop_depth (bb); which always yields zero if we don't have loops computed. So I can skip the computation only if we don't care about loop depths in early inlining either. Should I skip it? Thanks, Martin
> > This is not required to make hints working, it is necessary because of > the following line a in estimate_function_body_sizes: > > es->loop_depth = bb_loop_depth (bb); > > which always yields zero if we don't have loops computed. So I can > skip the computation only if we don't care about loop depths in early > inlining either. Should I skip it? Only place we care is the badness computation and only if profile guessing is off, so just initialize it to 0 for early inliner. Honza > > Thanks, > > Martin
Index: src/gcc/ipa-inline-analysis.c =================================================================== --- src.orig/gcc/ipa-inline-analysis.c +++ src/gcc/ipa-inline-analysis.c @@ -2102,6 +2102,11 @@ estimate_function_body_sizes (struct cgr info->conds = 0; info->entry = 0; + if (optimize) + { + calculate_dominance_info (CDI_DOMINATORS); + loop_optimizer_init (LOOPS_NORMAL | LOOPS_HAVE_RECORDED_EXITS); + } if (dump_file) fprintf (dump_file, "\nAnalyzing function body size: %s\n", @@ -2270,9 +2275,6 @@ estimate_function_body_sizes (struct cgr loop_iterator li; predicate loop_iterations = true_predicate (); - calculate_dominance_info (CDI_DOMINATORS); - loop_optimizer_init (LOOPS_NORMAL - | LOOPS_HAVE_RECORDED_EXITS); if (dump_file && (dump_flags & TDF_DETAILS)) flow_loops_dump (dump_file, NULL, 0); scev_initialize (); @@ -2305,12 +2307,15 @@ estimate_function_body_sizes (struct cgr *inline_summary (node)->loop_iterations = loop_iterations; } scev_finalize (); - loop_optimizer_finalize (); - free_dominance_info (CDI_DOMINATORS); } inline_summary (node)->self_time = time; inline_summary (node)->self_size = size; VEC_free (predicate_t, heap, nonconstant_names); + if (optimize) + { + loop_optimizer_finalize (); + free_dominance_info (CDI_DOMINATORS); + } if (dump_file) { fprintf (dump_file, "\n");