[PATCH] Fix profile count updates during tail merging

[Teresa Johnson]

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.

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;

@@ -1494,6 +1496,18 @@ replace_block_by (basic_block bb1, basic_block bb2

   bb2->count += bb1->count;

+  /* 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;
+      /* Recompute the probability from the new merged edge count (bb2->count
+         was updated above).  */
+      e2->probability = GCOV_COMPUTE_SCALE (e2->count, bb2->count);
+    }
+
   /* Do updates that use bb1, before deleting bb1.  */
   release_last_vdef (bb1);
   same_succ_flush_bb (bb1);



>
> Honza
>> +    }
>> +
>>    bb2->frequency += bb1->frequency;
>>    if (bb2->frequency > BB_FREQ_MAX)
>>      bb2->frequency = BB_FREQ_MAX;
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413