| Message ID | 20190110222559.GB30353@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix misplaced combine totals dumping (PR bootstrap/88714) | expand |
On Thu, Jan 10, 2019 at 11:25:59PM +0100, Jakub Jelinek wrote: > So, in the end, the combiner statistics was emitted in profile_estimate dump > and on the PR88714 issue suggested there is a difference already in the > profile_estimate dump, when actually the IL changed only during pre and of > course everything after it, including the combiner. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Seems obvious to me? Okay for trunk as far as combine is concerned :-) Segher > 2019-01-10 Jakub Jelinek <jakub@redhat.com> > > PR bootstrap/88714 > * passes.c (finish_optimization_passes): Call print_combine_total_stats > inside of pass_combine_1 dump rather than pass_profile_1. > > --- gcc/passes.c.jj 2019-01-01 12:37:15.494002253 +0100 > +++ gcc/passes.c 2019-01-10 16:30:43.295424173 +0100 > @@ -361,9 +361,9 @@ finish_optimization_passes (void) > > if (optimize > 0) > { > - dumps->dump_start (pass_profile_1->static_pass_number, NULL); > + dumps->dump_start (pass_combine_1->static_pass_number, NULL); > print_combine_total_stats (); > - dumps->dump_finish (pass_profile_1->static_pass_number); > + dumps->dump_finish (pass_combine_1->static_pass_number); > } > > /* Do whatever is necessary to finish printing the graphs. */ >
On January 10, 2019 11:25:59 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote: >Hi! > >r191883 seems to have introduced a pasto: >--- trunk/gcc/passes.c 2012/10/01 00:17:52 191882 >+++ trunk/gcc/passes.c 2012/10/01 05:43:06 191883 >@@ -231,27 +231,23 @@ > timevar_push (TV_DUMP); >if (profile_arc_flag || flag_test_coverage || >flag_branch_probabilities) > { >- dump_file = dump_begin (pass_profile.pass.static_pass_number, >NULL); >+ dump_start (pass_profile.pass.static_pass_number, NULL); > end_branch_prob (); >- if (dump_file) >- dump_end (pass_profile.pass.static_pass_number, dump_file); >+ dump_finish (pass_profile.pass.static_pass_number); > } > > if (optimize > 0) > { >- dump_file = dump_begin (pass_combine.pass.static_pass_number, >NULL); >- if (dump_file) >- { >- dump_combine_total_stats (dump_file); >- dump_end (pass_combine.pass.static_pass_number, dump_file); >- } >+ dump_start (pass_profile.pass.static_pass_number, NULL); >+ print_combine_total_stats (); >+ dump_finish (pass_combine.pass.static_pass_number); > } > >where dump_finish was used with correct pass_combine, but dump_start >was >pastoed from the previous if and contained pass_profile instead. >Next r193821 noticed this, but instead of fixing the dump_start >argument >changed dump_finish argument to match. > >So, in the end, the combiner statistics was emitted in profile_estimate >dump >and on the PR88714 issue suggested there is a difference already in the >profile_estimate dump, when actually the IL changed only during pre and >of >course everything after it, including the combiner. > >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Richard. >2019-01-10 Jakub Jelinek <jakub@redhat.com> > > PR bootstrap/88714 > * passes.c (finish_optimization_passes): Call >print_combine_total_stats > inside of pass_combine_1 dump rather than pass_profile_1. > >--- gcc/passes.c.jj 2019-01-01 12:37:15.494002253 +0100 >+++ gcc/passes.c 2019-01-10 16:30:43.295424173 +0100 >@@ -361,9 +361,9 @@ finish_optimization_passes (void) > > if (optimize > 0) > { >- dumps->dump_start (pass_profile_1->static_pass_number, NULL); >+ dumps->dump_start (pass_combine_1->static_pass_number, NULL); > print_combine_total_stats (); >- dumps->dump_finish (pass_profile_1->static_pass_number); >+ dumps->dump_finish (pass_combine_1->static_pass_number); > } > > /* Do whatever is necessary to finish printing the graphs. */ > > Jakub
--- trunk/gcc/passes.c 2012/10/01 00:17:52 191882 +++ trunk/gcc/passes.c 2012/10/01 05:43:06 191883 @@ -231,27 +231,23 @@ timevar_push (TV_DUMP); if (profile_arc_flag || flag_test_coverage || flag_branch_probabilities) { - dump_file = dump_begin (pass_profile.pass.static_pass_number, NULL); + dump_start (pass_profile.pass.static_pass_number, NULL); end_branch_prob (); - if (dump_file) - dump_end (pass_profile.pass.static_pass_number, dump_file); + dump_finish (pass_profile.pass.static_pass_number); } if (optimize > 0) { - dump_file = dump_begin (pass_combine.pass.static_pass_number, NULL); - if (dump_file) - { - dump_combine_total_stats (dump_file); - dump_end (pass_combine.pass.static_pass_number, dump_file); - } + dump_start (pass_profile.pass.static_pass_number, NULL); + print_combine_total_stats (); + dump_finish (pass_combine.pass.static_pass_number); } where dump_finish was used with correct pass_combine, but dump_start was pastoed from the previous if and contained pass_profile instead. Next r193821 noticed this, but instead of fixing the dump_start argument changed dump_finish argument to match. So, in the end, the combiner statistics was emitted in profile_estimate dump and on the PR88714 issue suggested there is a difference already in the profile_estimate dump, when actually the IL changed only during pre and of course everything after it, including the combiner. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2019-01-10 Jakub Jelinek <jakub@redhat.com> PR bootstrap/88714 * passes.c (finish_optimization_passes): Call print_combine_total_stats inside of pass_combine_1 dump rather than pass_profile_1. --- gcc/passes.c.jj 2019-01-01 12:37:15.494002253 +0100 +++ gcc/passes.c 2019-01-10 16:30:43.295424173 +0100 @@ -361,9 +361,9 @@ finish_optimization_passes (void) if (optimize > 0) { - dumps->dump_start (pass_profile_1->static_pass_number, NULL); + dumps->dump_start (pass_combine_1->static_pass_number, NULL); print_combine_total_stats (); - dumps->dump_finish (pass_profile_1->static_pass_number); + dumps->dump_finish (pass_combine_1->static_pass_number); } /* Do whatever is necessary to finish printing the graphs. */