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

login
register
mail settings
Submitter Tom de Vries
Date Oct. 30, 2011, 8:27 a.m.
Message ID <4EAD0A8C.4050603@mentor.com>
Download mbox | patch
Permalink /patch/122588/
State New
Headers show

Comments

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.
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

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);