Patchwork PR/44970, fix broken fwprop incremental dataflow

login
register
mail settings
Submitter Paolo Bonzini
Date Oct. 19, 2010, 2:08 p.m.
Message ID <4CBDA649.7040503@gnu.org>
Download mbox | patch
Permalink /patch/68338/
State New
Headers show

Comments

Paolo Bonzini - Oct. 19, 2010, 2:08 p.m.
On 10/19/2010 03:28 PM, Jack Howarth wrote:
> On Mon, Oct 18, 2010 at 11:27:43PM +0200, Paolo Bonzini wrote:
>> It turns out fwprop has been incredibly broken since it's inception.  :(
>>
>> Its incremental dataflow update looks at the uses in the instruction
>> before propagation, and creates new uses based on their presence after
>> propagation.  But when fwprop copy propagates a pseudo into another
>> instruction, it misses the uses of the propagated pseudo.  I find it
>> incredible that this never triggered in so many years.
>>
>> To fix the problem, the patch does not look at the old uses, and instead
>> reconstructs them using df-scan.  To do this it cannot use
>> df_insn_rescan, because this would remove the old defs and make the
>> use->def links dangling.  So I just created a new function
>> df_uses_create that scans an instruction and creates new refs like
>> df_ref_create would have done.
>>
>> With this in place, there is the question of how to create use->def
>> links for the new uses.  The old way to do the updates could easily
>> create the links because, by knowing a correspondence from old to new
>> uses, it could reuse each old use's def link.
>>
>> The new scheme, instead, is based on the following observation: the new
>> uses can only refer to very few pseudos, those in the propagated-from
>> insn and those in the propagated-to insn. fwprop walks the uses in the
>> two insns and prepares in advance a map from pseudos to their defs. When
>> checking is enabled, I added assertions that the values of the array
>> indeed were set for the right insns.
>>
>> Bootstrapped/regtested x86_64-pc-linux-gnu, bootstrapped on
>> hppa64-hp-hpux11.11 by Steve Ellcey.
>>
>> Ok for mainline?
> 
> On x86_64-apple-darwin10, this causes the regression of...
> 
> FAIL: gcc.c-torture/execute/arith-rand-ll.c compilation,  -Os  (internal compiler error)
> UNRESOLVED: gcc.c-torture/execute/arith-rand-ll.c execution,  -Os
> 
> at -m32.

Thanks, consider this squashed in:


Paolo
Jack Howarth - Oct. 19, 2010, 4:38 p.m.
On Tue, Oct 19, 2010 at 04:08:09PM +0200, Paolo Bonzini wrote:
> On 10/19/2010 03:28 PM, Jack Howarth wrote:
> > 
> > On x86_64-apple-darwin10, this causes the regression of...
> > 
> > FAIL: gcc.c-torture/execute/arith-rand-ll.c compilation,  -Os  (internal compiler error)
> > UNRESOLVED: gcc.c-torture/execute/arith-rand-ll.c execution,  -Os
> > 
> > at -m32.
> 
> Thanks, consider this squashed in:

Paolo,
   I can confirm that this fixes the issue on x86_64-apple-darwin10.
           Jack

> 
> Index: gcc/df-scan.c
> ===================================================================
> --- gcc/df-scan.c (revision 163855)
> +++ gcc/df-scan.c (working copy)
> @@ -2841,7 +2864,8 @@ df_ref_record (enum df_ref_class cl,
>        /*  If this is a multiword hardreg, we create some extra
>           datastructures that will enable us to easily build REG_DEAD
>           and REG_UNUSED notes.  */
> -      if ((endregno != regno + 1) && insn_info)
> +      if (collection_rec
> +         && (endregno != regno + 1) && insn_info)
>         {
>           /* Sets to a subreg of a multiword register are partial.
>              Sets to a non-subreg of a multiword register are not.  */
> 
> Paolo

Patch

Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c (revision 163855)
+++ gcc/df-scan.c (working copy)
@@ -2841,7 +2864,8 @@  df_ref_record (enum df_ref_class cl,
       /*  If this is a multiword hardreg, we create some extra
          datastructures that will enable us to easily build REG_DEAD
          and REG_UNUSED notes.  */
-      if ((endregno != regno + 1) && insn_info)
+      if (collection_rec
+         && (endregno != regno + 1) && insn_info)
        {
          /* Sets to a subreg of a multiword register are partial.
             Sets to a non-subreg of a multiword register are not.  */