[dataflow] partial register handling

[dataflow] partial register handling

[Roman Zippel]

Hi,

I have a few problems with the m68k mulsidi3 pattern on the dataflow
branch.
Currently incorrect code is generated as the DF_REF_PARTIAL bit isn't
set for its destinations and dataflow thinks both set the register
completely, thus one destination is set to unused. I could change the
pattern to include a strict_low_part, but that still wouldn't help due
to a bug in df_def_record_1().
Looking closer at this I don't think strict_low_part should be required
as splitting DI registers produces a lot of (subreg:SI (reg:DI)) even as
destination, but they only set strictly part of the register. If I look
through i386/m68k I don't see a single (strict_low_part (subreg:SI)).
I also think that ZERO_EXTRACT should set DF_REF_PARTIAL as well, since
e.g. these two patterns are equivalent:

(set (strict_low_part (subreg:HI (reg:SI))) ...)
(set (zero_extract:SI (reg:SI) (const_int 16) (const_int 16)) ...)

Below is the first attempt at fixing this, the subreg handling might
need further improvement. df_ref_record sets the partial bit for hard
registers but not for virtual registers?
I also removed one check from df_read_modify_subreg_p, why should
(subreg:QI (reg:DI)) be treated differently than (subreg:QI (reg:SI))? 
The df_read_modify_subreg_p might be taken out of the loop, as currently
df_ref_record sees only a fraction of all subregs and thus I'm not sure
the subreg check always works.

bye, Roman

---
 gcc/df-scan.c |   28 ------------!!!!!!!!!!!!!!!!
 1 file changed, 12 deletions(-), 16 modifications(!)

Index: gcc-dataflow/gcc/df-scan.c
===================================================================
*** gcc-dataflow.orig/gcc/df-scan.c
--- gcc-dataflow/gcc/df-scan.c
*************** df_read_modify_subreg_p (rtx x)
*** 2702,2708 ****
      return false;
    isize = GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)));
    osize = GET_MODE_SIZE (GET_MODE (x));
!   return (isize > osize && isize > UNITS_PER_WORD);
  }
  
  
--- 2702,2708 ----
      return false;
    isize = GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)));
    osize = GET_MODE_SIZE (GET_MODE (x));
!   return isize > osize;
  }
  
  
*************** df_def_record_1 (struct df_collection_re
*** 2717,2723 ****
  {
    rtx *loc;
    rtx dst;
-   bool dst_in_strict_lowpart = false;
  
   /* We may recursively call ourselves on EXPR_LIST when dealing with PARALLEL
       construct.  */
--- 2717,2722 ----
*************** df_def_record_1 (struct df_collection_re
*** 2751,2780 ****
  	 || GET_CODE (dst) == ZERO_EXTRACT
  	 || df_read_modify_subreg_p (dst))
      {
! #if 0
!       /* Strict low part always contains SUBREG, but we do not want to make
! 	 it appear outside, as whole register is always considered.  */
!       if (GET_CODE (dst) == STRICT_LOW_PART)
! 	{
! 	  loc = &XEXP (dst, 0);
! 	  dst = *loc;
! 	}
! #endif
        loc = &XEXP (dst, 0);
-       if (GET_CODE (dst) == STRICT_LOW_PART)
- 	dst_in_strict_lowpart = true;
        dst = *loc;
-       flags |= DF_REF_READ_WRITE;
- 
      }
  
-   /* Sets to a subreg of a single word register are partial sets if
-      they are wrapped in a strict lowpart, and not partial otherwise.
-   */
-   if (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))
-       && dst_in_strict_lowpart)
-     flags |= DF_REF_PARTIAL;
-     
    if (REG_P (dst)
        || (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))))
      df_ref_record (collection_rec, 
--- 2750,2764 ----
  	 || GET_CODE (dst) == ZERO_EXTRACT
  	 || df_read_modify_subreg_p (dst))
      {
!       flags |= DF_REF_READ_WRITE;
!       if (GET_CODE (dst) != SUBREG
! 	  || GET_MODE_SIZE (GET_MODE (dst))
! 	     >= REGMODE_NATURAL_SIZE (GET_MODE (dst)))
! 	flags |= DF_REF_PARTIAL;
        loc = &XEXP (dst, 0);
        dst = *loc;
      }
  
    if (REG_P (dst)
        || (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))))
      df_ref_record (collection_rec, 

Re: [dataflow] partial register handling

[Roman Zippel]

Hi,

On Thu, 10 May 2007, I wrote:

> I have a few problems with the m68k mulsidi3 pattern on the dataflow
> branch.

To illustrate the problem here is what happens during combine:

-(insn 7 28 8 2 ../gcc/gcc/testsuite/gcc.c-torture/execute/20001108-1.c:4 (parallel [
-            (set (subreg:SI (reg:DI 30 [ D.1547 ]) 4)
-                (mult:SI (reg/v:SI 33 [ x ])
-                    (subreg:SI (reg/v:DI 32 [ sum ]) 4)))
-            (set (subreg:SI (reg:DI 30 [ D.1547 ]) 0)
-                (truncate:SI (lshiftrt:DI (mult:DI (sign_extend:DI (reg/v:SI 33 [ x ]))
-                            (sign_extend:DI (subreg:SI (reg/v:DI 32 [ sum ]) 4)))
-                        (const_int 32 [0x20]))))
-        ]) 182 {*m68k.md:2733} (expr_list:REG_DEAD (reg/v:SI 33 [ x ])
-        (expr_list:REG_UNUSED (reg:DI 30 [ D.1547 ])
-            (nil))))
+(insn 7 28 8 2 ../gcc/gcc/testsuite/gcc.c-torture/execute/20001108-1.c:4 (set (subreg:SI (reg:DI 30 [ D.1547 ]) 4)
+        (mult:SI (mem/c/i:SI (plus:SI (reg/f:SI 24 %argptr)
+                    (const_int 16 [0x10])) [3 x+0 S4 A32])
+            (subreg:SI (reg/v:DI 32 [ sum ]) 4))) 176 {*m68k.md:2643} (expr_list:REG_UNUSED (reg:DI 30 [ D.1547 ])
+        (nil)))

The REG_UNUSED note is wrong and thus combine is confused.
Using strict_low_part doesn't make a difference, as DF_REF_PARTIAL isn't 
set in this case either, so df thinks the first set is overwritten by 
second one and generates this note.

> Currently incorrect code is generated as the DF_REF_PARTIAL bit isn't
> set for its destinations and dataflow thinks both set the register
> completely, thus one destination is set to unused. I could change the
> pattern to include a strict_low_part, but that still wouldn't help due
> to a bug in df_def_record_1().

It may help my understanding of df_def_record_1() to have a few examples 
of how the flags should be correctly set (for a 32 bit port):

(set (subreg:SI (reg:DI) 4) ...
DF_REF_READ_WRITE|DF_REF_PARTIAL?

(set (subreg:HI (reg:DI) 6) ...
DF_REF_READ_WRITE?

(set (subreg:HI (reg:SI) 2) ...
DF_REF_READ_WRITE?

(set (strict_low_part (subreg:HI (reg:DI) 6)) ...
DF_REF_READ_WRITE|DF_REF_PARTIAL?

(set (strict_low_part (subreg:HI (reg:SI) 2)) ...
DF_REF_READ_WRITE|DF_REF_PARTIAL?

(set (zero_extract:SI (reg:SI) (const_int 16) (const_int 16)) ...)
DF_REF_READ_WRITE|DF_REF_PARTIAL?

The basic question is maybe how a partial write should exactly be handled.

bye, Roman

Re: [dataflow] partial register handling

[Rask Ingemann Lambertsen]

On Fri, May 11, 2007 at 01:54:24PM +0200, Roman Zippel wrote:
> 
> To illustrate the problem here is what happens during combine:
> 
> -(insn 7 28 8 2 ../gcc/gcc/testsuite/gcc.c-torture/execute/20001108-1.c:4 (parallel [
> -            (set (subreg:SI (reg:DI 30 [ D.1547 ]) 4)
> -                (mult:SI (reg/v:SI 33 [ x ])
> -                    (subreg:SI (reg/v:DI 32 [ sum ]) 4)))
> -            (set (subreg:SI (reg:DI 30 [ D.1547 ]) 0)
> -                (truncate:SI (lshiftrt:DI (mult:DI (sign_extend:DI (reg/v:SI 33 [ x ]))
> -                            (sign_extend:DI (subreg:SI (reg/v:DI 32 [ sum ]) 4)))
> -                        (const_int 32 [0x20]))))
> -        ]) 182 {*m68k.md:2733} (expr_list:REG_DEAD (reg/v:SI 33 [ x ])
> -        (expr_list:REG_UNUSED (reg:DI 30 [ D.1547 ])
> -            (nil))))
> +(insn 7 28 8 2 ../gcc/gcc/testsuite/gcc.c-torture/execute/20001108-1.c:4 (set (subreg:SI (reg:DI 30 [ D.1547 ]) 4)
> +        (mult:SI (mem/c/i:SI (plus:SI (reg/f:SI 24 %argptr)
> +                    (const_int 16 [0x10])) [3 x+0 S4 A32])
> +            (subreg:SI (reg/v:DI 32 [ sum ]) 4))) 176 {*m68k.md:2643} (expr_list:REG_UNUSED (reg:DI 30 [ D.1547 ])
> +        (nil)))

   The first one is the insn pattern right below the mulsidi3 expander,
   right? Please give all insn patterns a name to make searches easier.

   May I ask why the original insn 7 isn't coded something like

(set (reg:DI 30) (mult:DI (sign_extend:DI (reg:SI 33))
			  (sign_extend:DI (reg:SI 32))))

instead?

-- 
Rask Ingemann Lambertsen

Re: [dataflow] partial register handling

[Kenneth Zadeck]

Roman Zippel wrote:
> Hi,
>
> On Thu, 10 May 2007, I wrote:
>
>   
>> I have a few problems with the m68k mulsidi3 pattern on the dataflow
>> branch.
>>     
>
> To illustrate the problem here is what happens during combine:
>
> -(insn 7 28 8 2 ../gcc/gcc/testsuite/gcc.c-torture/execute/20001108-1.c:4 (parallel [
> -            (set (subreg:SI (reg:DI 30 [ D.1547 ]) 4)
> -                (mult:SI (reg/v:SI 33 [ x ])
> -                    (subreg:SI (reg/v:DI 32 [ sum ]) 4)))
> -            (set (subreg:SI (reg:DI 30 [ D.1547 ]) 0)
> -                (truncate:SI (lshiftrt:DI (mult:DI (sign_extend:DI (reg/v:SI 33 [ x ]))
> -                            (sign_extend:DI (subreg:SI (reg/v:DI 32 [ sum ]) 4)))
> -                        (const_int 32 [0x20]))))
> -        ]) 182 {*m68k.md:2733} (expr_list:REG_DEAD (reg/v:SI 33 [ x ])
> -        (expr_list:REG_UNUSED (reg:DI 30 [ D.1547 ])
> -            (nil))))
> +(insn 7 28 8 2 ../gcc/gcc/testsuite/gcc.c-torture/execute/20001108-1.c:4 (set (subreg:SI (reg:DI 30 [ D.1547 ]) 4)
> +        (mult:SI (mem/c/i:SI (plus:SI (reg/f:SI 24 %argptr)
> +                    (const_int 16 [0x10])) [3 x+0 S4 A32])
> +            (subreg:SI (reg/v:DI 32 [ sum ]) 4))) 176 {*m68k.md:2643} (expr_list:REG_UNUSED (reg:DI 30 [ D.1547 ])
> +        (nil)))
>
> The REG_UNUSED note is wrong and thus combine is confused.
> Using strict_low_part doesn't make a difference, as DF_REF_PARTIAL isn't 
> set in this case either, so df thinks the first set is overwritten by 
> second one and generates this note.
>
>   
Roman,

There was a debate several months ago on this issue: how much should the
df scanner be a theorem prover with respect to how many bits survive an
operation.
For instance, I could easily add to your list, anding and oring
operations which also preserve bits. 

The decision was made that what is done in the dataflow scanner was
sufficient and that the machine descriptions should conform to this. 

If you have a pattern that cannot be made to conform to what the df
scanner currently does, then we will certainly reopen this issue.  But
the consensus was at the time that this was sufficient and that we did
not want to further burden the scanner. 

While I could be argued that this is an arbitrary decision, it is
important to keep the scanning fast and there is a lot of downstream
benefit to imposing some consistency on the md's coding style.

Kenny

>> Currently incorrect code is generated as the DF_REF_PARTIAL bit isn't
>> set for its destinations and dataflow thinks both set the register
>> completely, thus one destination is set to unused. I could change the
>> pattern to include a strict_low_part, but that still wouldn't help due
>> to a bug in df_def_record_1().
>>     
>
> It may help my understanding of df_def_record_1() to have a few examples 
> of how the flags should be correctly set (for a 32 bit port):
>
> (set (subreg:SI (reg:DI) 4) ...
> DF_REF_READ_WRITE|DF_REF_PARTIAL?
>
> (set (subreg:HI (reg:DI) 6) ...
> DF_REF_READ_WRITE?
>
> (set (subreg:HI (reg:SI) 2) ...
> DF_REF_READ_WRITE?
>
> (set (strict_low_part (subreg:HI (reg:DI) 6)) ...
> DF_REF_READ_WRITE|DF_REF_PARTIAL?
>
> (set (strict_low_part (subreg:HI (reg:SI) 2)) ...
> DF_REF_READ_WRITE|DF_REF_PARTIAL?
>
> (set (zero_extract:SI (reg:SI) (const_int 16) (const_int 16)) ...)
> DF_REF_READ_WRITE|DF_REF_PARTIAL?
>
> The basic question is maybe how a partial write should exactly be handled.
>
> bye, Roman
>   

Re: [dataflow] partial register handling

[Rask Ingemann Lambertsen]

On Thu, May 10, 2007 at 07:43:19PM +0200, Roman Zippel wrote:

> Looking closer at this I don't think strict_low_part should be required
> as splitting DI registers produces a lot of (subreg:SI (reg:DI)) even as
> destination, but they only set strictly part of the register. If I look
> through i386/m68k I don't see a single (strict_low_part (subreg:SI)).

   That's because with UNITS_PER_WORD == 4, (subreg:SI (reg:DI) n) is
implicitly a strict low part for n == 0 or n == 4. When used as destination,
such a subreg is defined as not clobbering anything outside the subreg.

-- 
Rask Ingemann Lambertsen

Re: [dataflow] partial register handling

[Paolo Bonzini]

My opinion:

>> (set (subreg:SI (reg:DI) 4) ...
>> DF_REF_READ_WRITE|DF_REF_PARTIAL?

yep

>> (set (subreg:HI (reg:DI) 6) ...
>> DF_REF_READ_WRITE?

Also PARTIAL.  you can rely on the content of the 0-3 word.

Likewise for:

   (set (subreg:HI (reg:DI) 2) ...

here you can rely on the content of the 4-7 word.

>> (set (subreg:HI (reg:SI) 2) ...
>> DF_REF_READ_WRITE?

yep, only case without PARTIAL

>> (set (strict_low_part (subreg:HI (reg:DI) 6)) ...
>> DF_REF_READ_WRITE|DF_REF_PARTIAL?

yep

>> (set (strict_low_part (subreg:HI (reg:SI) 2)) ...
>> DF_REF_READ_WRITE|DF_REF_PARTIAL?

yep

>> (set (zero_extract:SI (reg:SI) (const_int 16) (const_int 16)) ...)
>> DF_REF_READ_WRITE|DF_REF_PARTIAL?

yep

Re: [dataflow] partial register handling

[Roman Zippel]

Hi,

On Fri, 11 May 2007, Rask Ingemann Lambertsen wrote:

>    The first one is the insn pattern right below the mulsidi3 expander,
>    right? Please give all insn patterns a name to make searches easier.

It's on the TODO list. :)

>    May I ask why the original insn 7 isn't coded something like
> 
> (set (reg:DI 30) (mult:DI (sign_extend:DI (reg:SI 33))
> 			  (sign_extend:DI (reg:SI 32))))
> 
> instead?

In the long term we want to expose the subreg for better register 
allocation. It would be possible to initially generate this and split it 
later (this would also avoid this problem), but this makes only more 
complicated. So I'm a little hesitant to do this, if we can just generate 
the final form immediately.

bye, Roman

Re: [dataflow] partial register handling

[Paolo Bonzini]

First of all, scrap my other message...

> There was a debate several months ago on this issue: how much should the
> df scanner be a theorem prover with respect to how many bits survive an
> operation.
> For instance, I could easily add to your list, anding and oring
> operations which also preserve bits. 

The rules should be simple.  Bits of the dest reg survive only if one of 
the following is true.

- there is a STRICT_LOW_PART (of a SUBREG)

- there is a ZERO_EXTRACT (not necessarily of a SUBREG!)

- the subreg is part of a multiword subreg

The last point is decided in other parts of the code than the one Roman 
is touching.  So, Roman's change to df_read_modify_subreg_p is wrong; 
other subregs, in particular (subreg:HI (reg:SI 123) 2), should not be 
affected.  At most, we might want there the more pedantic

   return (isize > osize
           && isize > REGMODE_NATURAL_SIZE (GET_MODE (isize));


It seems to me that all is missing is setting DF_REF_PARTIAL for 
ZERO_EXTRACT.  That is

-      if (GET_CODE (dst) == STRICT_LOW_PART)
-        dst_in_strict_lowpart = true;
+      if (GET_CODE (dst) == STRICT_LOW_PART
+          || GET_CODE (dst) == ZERO_EXTRACT)
+        flags |= DF_REF_PARTIAL;

Paolo

Re: [dataflow] partial register handling

[Roman Zippel]

Hi,

On Fri, 11 May 2007, Kenneth Zadeck wrote:

> There was a debate several months ago on this issue: how much should the
> df scanner be a theorem prover with respect to how many bits survive an
> operation.
> For instance, I could easily add to your list, anding and oring
> operations which also preserve bits. 

I think that's a different problem, as I don't want to know which bits 
exactly survive. It's about the information that something survived. If we 
can't or don't want to prove what survives we can't eliminate previous 
operations.
The problem here is that scanner IMO incorrectly thinks that nothing 
survives (by not setting the partial bit).

bye, Roman

Re: [dataflow] partial register handling

[Roman Zippel]

Hi,

On Fri, 11 May 2007, Paolo Bonzini wrote:

> First of all, scrap my other message...
> 
> > There was a debate several months ago on this issue: how much should the
> > df scanner be a theorem prover with respect to how many bits survive an
> > operation.
> > For instance, I could easily add to your list, anding and oring
> > operations which also preserve bits. 
> 
> The rules should be simple.  Bits of the dest reg survive only if one of the
> following is true.
> 
> - there is a STRICT_LOW_PART (of a SUBREG)
> 
> - there is a ZERO_EXTRACT (not necessarily of a SUBREG!)
> 
> - the subreg is part of a multiword subreg
> 
> The last point is decided in other parts of the code than the one Roman is
> touching.  So, Roman's change to df_read_modify_subreg_p is wrong; other
> subregs, in particular (subreg:HI (reg:SI 123) 2), should not be affected.  At
> most, we might want there the more pedantic
> 
>   return (isize > osize
>           && isize > REGMODE_NATURAL_SIZE (GET_MODE (isize));
> 
> 
> It seems to me that all is missing is setting DF_REF_PARTIAL for ZERO_EXTRACT.
> That is
> 
> -      if (GET_CODE (dst) == STRICT_LOW_PART)
> -        dst_in_strict_lowpart = true;
> +      if (GET_CODE (dst) == STRICT_LOW_PART
> +          || GET_CODE (dst) == ZERO_EXTRACT)
> +        flags |= DF_REF_PARTIAL;

That clears up about df_read_modify_subreg_p, thanks.
But I don't think that's enough, with the current loop it would strip the 
subreg of a multiword subreg and there is some logic in df_ref_record, 
which wouldn't see it. An alternative might be:

  while (GET_CODE (dst) == STRICT_LOW_PART
	 || GET_CODE (dst) == ZERO_EXTRACT)
    {
      flags |= DF_REF_READ_WRITE | DF_REF_PARTIAL;
      loc = &XEXP (dst, 0);
      dst = *loc;
    }

  if (df_read_modify_subreg_p (dst))
    flags |= DF_REF_READ_WRITE | DF_REF_PARTIAL;

That would only leave singleword subreg and paradoxical subreg, if they 
should require anything.
One could also restrict STRICT_LOW_PART to subregs, as after reload the 
subreg part is usually gone.

bye, Roman

Re: [dataflow] partial register handling

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]


> The rules should be simple.  Bits of the dest reg survive only if one of 
> the following is true.
> 
> - there is a STRICT_LOW_PART (of a SUBREG)
> 
> - there is a ZERO_EXTRACT (not necessarily of a SUBREG!)
> 
> - the subreg is part of a multiword subreg

So, this is a prototype patch that I would like to go in.

Paolo

[-- Attachment #2: subregs.patch --]
[-- Type: text/plain, Size: 2182 bytes --]

	* df-scan.c (df_read_modify_subreg_p): Use REGMODE_NATURAL_SIZE.
	(df_def_record_1): Sets to ZERO_EXTRACT get DF_REF_PARTIAL.

Index: gcc-test-df/base-gcc-src/gcc/df-scan.c
===================================================================
--- gcc-test-df/base-gcc-src/gcc/df-scan.c	(revision 124539)
+++ gcc-test-df/base-gcc-src/gcc/df-scan.c	(working copy)
@@ -2697,12 +2697,15 @@ df_ref_record (struct df_collection_rec 
 bool
 df_read_modify_subreg_p (rtx x)
 {
+  enum machine_mode imode, omode;
   unsigned int isize, osize;
   if (GET_CODE (x) != SUBREG)
     return false;
-  isize = GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)));
-  osize = GET_MODE_SIZE (GET_MODE (x));
-  return (isize > osize && isize > UNITS_PER_WORD);
+  imode = GET_MODE (SUBREG_REG (x));
+  omode = GET_MODE (x);
+  isize = GET_MODE_SIZE (imode);
+  osize = GET_MODE_SIZE (omode);
+  return (isize > osize && isize > REGMODE_NATURAL_SIZE (imode));
 }
 
 
@@ -2751,30 +2754,18 @@ df_def_record_1 (struct df_collection_re
 	 || GET_CODE (dst) == ZERO_EXTRACT
 	 || df_read_modify_subreg_p (dst))
     {
-#if 0
-      /* Strict low part always contains SUBREG, but we do not want to make
-	 it appear outside, as whole register is always considered.  */
-      if (GET_CODE (dst) == STRICT_LOW_PART)
-	{
-	  loc = &XEXP (dst, 0);
-	  dst = *loc;
-	}
-#endif
-      loc = &XEXP (dst, 0);
-      if (GET_CODE (dst) == STRICT_LOW_PART)
-	dst_in_strict_lowpart = true;
-      dst = *loc;
       flags |= DF_REF_READ_WRITE;
 
+      /* Apart from sets to a subreg of a multi-word register, all
+         partial sets are recognized here.  */
+      if (GET_CODE (dst) == STRICT_LOW_PART
+	  || GET_CODE (dst) == ZERO_EXTRACT)
+	flags |= DF_REF_PARTIAL;
+
+      loc = &XEXP (dst, 0);
+      dst = *loc;
     }
 
-  /* Sets to a subreg of a single word register are partial sets if
-     they are wrapped in a strict lowpart, and not partial otherwise.
-  */
-  if (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))
-      && dst_in_strict_lowpart)
-    flags |= DF_REF_PARTIAL;
-    
   if (REG_P (dst)
       || (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))))
     df_ref_record (collection_rec, 

Re: [dataflow] partial register handling

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]


> The rules should be simple.  Bits of the dest reg survive only if one of 
> the following is true.
> 
> - there is a STRICT_LOW_PART (of a SUBREG)
> 
> - there is a ZERO_EXTRACT (not necessarily of a SUBREG!)
> 
> - the subreg is part of a multiword subreg

So, this is a prototype patch that I would like to go in.

Paolo

[-- Attachment #2: subregs.patch --]
[-- Type: text/plain, Size: 2182 bytes --]

	* df-scan.c (df_read_modify_subreg_p): Use REGMODE_NATURAL_SIZE.
	(df_def_record_1): Sets to ZERO_EXTRACT get DF_REF_PARTIAL.

Index: gcc-test-df/base-gcc-src/gcc/df-scan.c
===================================================================
--- gcc-test-df/base-gcc-src/gcc/df-scan.c	(revision 124539)
+++ gcc-test-df/base-gcc-src/gcc/df-scan.c	(working copy)
@@ -2697,12 +2697,15 @@ df_ref_record (struct df_collection_rec 
 bool
 df_read_modify_subreg_p (rtx x)
 {
+  enum machine_mode imode, omode;
   unsigned int isize, osize;
   if (GET_CODE (x) != SUBREG)
     return false;
-  isize = GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)));
-  osize = GET_MODE_SIZE (GET_MODE (x));
-  return (isize > osize && isize > UNITS_PER_WORD);
+  imode = GET_MODE (SUBREG_REG (x));
+  omode = GET_MODE (x);
+  isize = GET_MODE_SIZE (imode);
+  osize = GET_MODE_SIZE (omode);
+  return (isize > osize && isize > REGMODE_NATURAL_SIZE (imode));
 }
 
 
@@ -2751,30 +2754,18 @@ df_def_record_1 (struct df_collection_re
 	 || GET_CODE (dst) == ZERO_EXTRACT
 	 || df_read_modify_subreg_p (dst))
     {
-#if 0
-      /* Strict low part always contains SUBREG, but we do not want to make
-	 it appear outside, as whole register is always considered.  */
-      if (GET_CODE (dst) == STRICT_LOW_PART)
-	{
-	  loc = &XEXP (dst, 0);
-	  dst = *loc;
-	}
-#endif
-      loc = &XEXP (dst, 0);
-      if (GET_CODE (dst) == STRICT_LOW_PART)
-	dst_in_strict_lowpart = true;
-      dst = *loc;
       flags |= DF_REF_READ_WRITE;
 
+      /* Apart from sets to a subreg of a multi-word register, all
+         partial sets are recognized here.  */
+      if (GET_CODE (dst) == STRICT_LOW_PART
+	  || GET_CODE (dst) == ZERO_EXTRACT)
+	flags |= DF_REF_PARTIAL;
+
+      loc = &XEXP (dst, 0);
+      dst = *loc;
     }
 
-  /* Sets to a subreg of a single word register are partial sets if
-     they are wrapped in a strict lowpart, and not partial otherwise.
-  */
-  if (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))
-      && dst_in_strict_lowpart)
-    flags |= DF_REF_PARTIAL;
-    
   if (REG_P (dst)
       || (GET_CODE (dst) == SUBREG && REG_P (SUBREG_REG (dst))))
     df_ref_record (collection_rec, 

Re: [dataflow] partial register handling

[Richard Sandiford]

Paolo Bonzini <bonzini@gnu.org> writes:
> My opinion:
>>> (set (subreg:HI (reg:SI) 2) ...
>>> DF_REF_READ_WRITE?
>
> yep, only case without PARTIAL

Sorry if this is a daft question, but why would it be treated as a read?
I thought that without strict_lowpart, this sort of subreg assignment
was morally equivalent to a normal REG definition in which the upper
bits are "don't care".

Richard

Re: [dataflow] partial register handling

[Paolo Bonzini]

Richard Sandiford wrote:
> Paolo Bonzini <bonzini@gnu.org> writes:
>> My opinion:
>>>> (set (subreg:HI (reg:SI) 2) ...
>>>> DF_REF_READ_WRITE?
>> yep, only case without PARTIAL
> 
> Sorry if this is a daft question, but why would it be treated as a read?
> I thought that without strict_lowpart, this sort of subreg assignment
> was morally equivalent to a normal REG definition in which the upper
> bits are "don't care".

Yes, I said elsewhere to ignore this message...

Paolo

Re: [dataflow] partial register handling

[Paolo Bonzini]

> But I don't think that's enough, with the current loop it would strip the 
> subreg of a multiword subreg and there is some logic in df_ref_record, 
> which wouldn't see it. An alternative might be:
> 
>   while (GET_CODE (dst) == STRICT_LOW_PART
> 	 || GET_CODE (dst) == ZERO_EXTRACT)
>     {
>       flags |= DF_REF_READ_WRITE | DF_REF_PARTIAL;
>       loc = &XEXP (dst, 0);
>       dst = *loc;
>     }
> 
>   if (df_read_modify_subreg_p (dst))
>     flags |= DF_REF_READ_WRITE | DF_REF_PARTIAL;
> 
> That would only leave singleword subreg and paradoxical subreg, if they 
> should require anything.

This seems also correct, if you want to test it.

> One could also restrict STRICT_LOW_PART to subregs, as after reload the 
> subreg part is usually gone.

No, a strict_low_part of a reg is also a partial set.

Paolo