[PR50878] Fix for verify_dominators in -ftree-tail-merge

Submitted by Tom de Vries on Oct. 30, 2011, 8:27 a.m.

Details

Message ID 4EAD0A8C.4050603@mentor.com
State New
Headers show

Commit Message

Tom de Vries Oct. 30, 2011, 8:27 a.m.
On 10/30/2011 09:20 AM, Tom de Vries wrote:
> Richard,
> 
> I have a fix for PR50878.

Sorry, with patch this time.

Thanks,
- Tom

> 
> A simplified form of the problem from the test-case of the PR is shown in this
> cfg. Block 12 has as direct dominator block 5.
> 
>         5
>        / \
>       /   \
>      *     *
>      6     7
>      |     |
>      |     |
>      *     *
>      8     9
>       \   /
>        \ /
>         *
>        12
> 
> tail_merge_optimize finds that blocks 6 and 7 are duplicates. After replacing
> block 7 by block 6, the cfg looks like this:
> 
>         5
>         |
>         |
>         *
>         6
>        / \
>       /   \
>      *     *
>      8     9
>       \   /
>        \ /
>         *
>        12
> 
> The new direct dominator of block 12 is block 6, but the current algorithm only
> recalculates dominator info for blocks 6, 8 and 9.
> 
> The patch fixes this by additionally recalculating the dominator info for blocks
> immediately dominated by bb2 (block 6 in the example), if bb2 has a single
> predecessor after replacement.
> 
> Bootstapped and reg-tested on x86_64 and i686. Build and reg-tested on MIPS and ARM.
> 
> Ok for trunk?
> 
> Thanks,
> - Tom
> 
> 2011-10-30  Tom de Vries  <tom@codesourcery.com>
> 
> 	PR tree-optimization/50878
> 	* tree-ssa-tail-merge.c (replace_block_by): Recalculate dominator info
> 	for blocks immediately dominated by bb2, if bb2 has a single predecessor
> 	after replacement.

Comments

Richard Guenther Oct. 30, 2011, 9:54 a.m.
On Sun, Oct 30, 2011 at 9:27 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 10/30/2011 09:20 AM, Tom de Vries wrote:
>> Richard,
>>
>> I have a fix for PR50878.
>
> Sorry, with patch this time.

Ok for now, but see Davids mail and the complexity issue with iteratively
updating dominators.  It seems to me that we know exactly what to update
and how, and we should do that (well, if we need up-to-date dominators,
re-computing them once in the pass would be ok).

Richard.

> Thanks,
> - Tom
>
>>
>> A simplified form of the problem from the test-case of the PR is shown in this
>> cfg. Block 12 has as direct dominator block 5.
>>
>>         5
>>        / \
>>       /   \
>>      *     *
>>      6     7
>>      |     |
>>      |     |
>>      *     *
>>      8     9
>>       \   /
>>        \ /
>>         *
>>        12
>>
>> tail_merge_optimize finds that blocks 6 and 7 are duplicates. After replacing
>> block 7 by block 6, the cfg looks like this:
>>
>>         5
>>         |
>>         |
>>         *
>>         6
>>        / \
>>       /   \
>>      *     *
>>      8     9
>>       \   /
>>        \ /
>>         *
>>        12
>>
>> The new direct dominator of block 12 is block 6, but the current algorithm only
>> recalculates dominator info for blocks 6, 8 and 9.
>>
>> The patch fixes this by additionally recalculating the dominator info for blocks
>> immediately dominated by bb2 (block 6 in the example), if bb2 has a single
>> predecessor after replacement.
>>
>> Bootstapped and reg-tested on x86_64 and i686. Build and reg-tested on MIPS and ARM.
>>
>> Ok for trunk?
>>
>> Thanks,
>> - Tom
>>
>> 2011-10-30  Tom de Vries  <tom@codesourcery.com>
>>
>>       PR tree-optimization/50878
>>       * tree-ssa-tail-merge.c (replace_block_by): Recalculate dominator info
>>       for blocks immediately dominated by bb2, if bb2 has a single predecessor
>>       after replacement.
>
>

Patch hide | download patch | download mbox

Index: gcc/tree-ssa-tail-merge.c
===================================================================
--- gcc/tree-ssa-tail-merge.c (revision 180562)
+++ gcc/tree-ssa-tail-merge.c (working copy)
@@ -1604,7 +1604,10 @@  replace_block_by (basic_block bb1, basic
   same_succ_flush_bb (bb1);
   delete_basic_block (bb1);
 
-  fix_dom_bb = VEC_alloc (basic_block, heap, 2);
+  if (single_pred_p (bb2))
+    fix_dom_bb = get_dominated_by (CDI_DOMINATORS, single_pred (bb2));
+  else
+    fix_dom_bb = VEC_alloc (basic_block, heap, 2);
   VEC_safe_push (basic_block, heap, fix_dom_bb, bb2);
   FOR_EACH_EDGE (e, ei, bb2->succs)
     VEC_safe_push (basic_block, heap, fix_dom_bb, e->dest);