This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix profile count updates during tail merging
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, David Li <davidxl at google dot com>
- Date: Fri, 18 Oct 2013 16:18:08 +0200
- Subject: Re: [PATCH] Fix profile count updates during tail merging
- Authentication-results: sourceware.org; auth=none
- References: <CAAe5K+UGneuko=W_UDb9AZ63TQE7O1Ryk2=hwB7h5qBM1QKdAw at mail dot gmail dot com> <20131015210522 dot GB23980 at kam dot mff dot cuni dot cz> <CAAe5K+UDEp-bq7RnRwsDjUPv3Fc9JoKAOq059FzvkfgDMeKy5A at mail dot gmail dot com>
> On Tue, Oct 15, 2013 at 2:05 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> This patch fixes a profile count insanity introduced by ssa tail
> >> merging. When replacing bb1 with bb2, which has the same successors,
> >> the bb counts were being merged, but the successor edge weights
> >> were not.
> >>
> >> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
> >>
> >> Thanks,
> >> Teresa
> >>
> >> 2013-10-15 Teresa Johnson <tejohnson@google.com>
> >>
> >> * tree-ssa-tail-merge.c (replace_block_by): Update edge
> >> weights during merging.
> >>
> >> Index: tree-ssa-tail-merge.c
> >> ===================================================================
> >> --- tree-ssa-tail-merge.c (revision 203389)
> >> +++ tree-ssa-tail-merge.c (working copy)
> >> @@ -1462,6 +1462,8 @@ static void
> >> replace_block_by (basic_block bb1, basic_block bb2)
> >> {
> >> edge pred_edge;
> >> + edge e1;
> >> + edge_iterator ei;
> >> unsigned int i;
> >> gimple bb2_phi;
> >>
> >> @@ -1488,6 +1490,15 @@ replace_block_by (basic_block bb1, basic_block bb2
> >> pred_edge, UNKNOWN_LOCATION);
> >> }
> >>
> >> + /* Merge the outgoing edge counts from bb1 onto bb2. */
> >> + FOR_EACH_EDGE (e1, ei, bb1->succs)
> >> + {
> >> + edge e2;
> >> + e2 = find_edge (bb2, e1->dest);
> >> + gcc_assert (e2);
> >> + e2->count += e1->count;
> >
> > Don't you need to redistribute the counts via edge probabilities?
>
> Good point on the probabilities. However, we want to redistribute the
> counts as-is and instead update the probabilities to reflect the new
> counts. Otherwise, successor blocks would need to have their counts
> updated.
>
> Here is the new patch, which was bootstrapped and tested on
> x86_64-unknown-linux-gnu. Ok for trunk?
>
> 2013-10-18 Teresa Johnson <tejohnson@google.com>
>
> * tree-ssa-tail-merge.c (replace_block_by): Update edge
> weights during merging.
This is OK, thanks!
Honza