Make more use of df_read_modify_subreg_p

Message ID 87a82qv9sw.fsf@linaro.org
State New
Headers show

Commit Message

Richard Sandiford Aug. 23, 2017, 10:49 a.m.
This patch uses df_read_modify_subreg_p to check whether writing
to a subreg would preserve some of the existing contents.

This has the effect of putting more emphasis on the
REGMODE_NATURAL_SIZE-based definition of whether something can be
partially modified, instead of using UNITS_PER_WORD unconditionally.
This becomes important for SVE, where UNITS_PER_WORD has no
significance for subregs of multi-register LD2/ST2, LD3/ST3 and
LD4/ST4 tuples.

Tested on aarch64-linux-gnu and x86_64-linux-gnu, and by making sure
that there were no differences in testsuite assembly output for one
target per CPU.  OK to install?

Richard


2017-08-23  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* df.h (df_read_modify_subreg_p): Take a const_rtx.
	* df-scan.c (df_read_modify_subreg_p): Likewise.
	* caller-save.c (mark_referenced_regs):  Use dr_read_modify_subreg_p.
	* combine.c (find_single_use_1): Likewise.
	(expand_field_assignment): Likewise.
	(move_deaths): Likewise.
	* lra-constraints.c (simplify_operand_subreg): Likewise.
	(curr_insn_transform): Likewise.
	* lra.c (collect_non_operand_hard_regs): Likewise.
	(add_regs_to_insn_regno_info): Likewise.
	* rtlanal.c (reg_referenced_p): Likewise.
	(covers_regno_no_parallel_p): Likewise.

Comments

Segher Boessenkool Aug. 23, 2017, 3:51 p.m. | #1
On Wed, Aug 23, 2017 at 11:49:03AM +0100, Richard Sandiford wrote:
> This patch uses df_read_modify_subreg_p to check whether writing

> to a subreg would preserve some of the existing contents.


combine does not keep the DF info up-to-date -- but that is no
problem here, df_read_modify_subreg_p uses no DF info at all.  Maybe
it should not have "df_" in the name?


Segher
Richard Sandiford Aug. 24, 2017, 6:25 p.m. | #2
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Wed, Aug 23, 2017 at 11:49:03AM +0100, Richard Sandiford wrote:

>> This patch uses df_read_modify_subreg_p to check whether writing

>> to a subreg would preserve some of the existing contents.

>

> combine does not keep the DF info up-to-date -- but that is no

> problem here, df_read_modify_subreg_p uses no DF info at all.  Maybe

> it should not have "df_" in the name?


Yeah, I guess that's a bit confusing.  I've just posted a patch
to rename it.

Here's a version of the patch that applies on top of that one.
Tested as before.  OK to install?

Thanks,
Richard


2017-08-24  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* caller-save.c (mark_referenced_regs):  Use read_modify_subreg_p.
	* combine.c (find_single_use_1): Likewise.
	(expand_field_assignment): Likewise.
	(move_deaths): Likewise.
	* lra-constraints.c (simplify_operand_subreg): Likewise.
	(curr_insn_transform): Likewise.
	* lra.c (collect_non_operand_hard_regs): Likewise.
	(add_regs_to_insn_regno_info): Likewise.
	* rtlanal.c (reg_referenced_p): Likewise.
	(covers_regno_no_parallel_p): Likewise.

Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	2017-08-24 19:14:37.901164512 +0100
+++ gcc/caller-save.c	2017-08-24 19:22:45.217100872 +0100
@@ -1033,10 +1033,7 @@ mark_referenced_regs (rtx *loc, refmarke
 	      /* If we're setting only part of a multi-word register,
 		 we shall mark it as referenced, because the words
 		 that are not being set should be restored.  */
-	      && ((GET_MODE_SIZE (GET_MODE (*loc))
-		   >= GET_MODE_SIZE (GET_MODE (SUBREG_REG (*loc))))
-		  || (GET_MODE_SIZE (GET_MODE (SUBREG_REG (*loc)))
-		      <= UNITS_PER_WORD))))
+	      && !read_modify_subreg_p (*loc)))
 	return;
     }
   if (code == MEM || code == SUBREG)
Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2017-08-24 19:22:26.163269637 +0100
+++ gcc/combine.c	2017-08-24 19:22:45.218100970 +0100
@@ -579,10 +579,7 @@ find_single_use_1 (rtx dest, rtx *loc)
 	  && !REG_P (SET_DEST (x))
 	  && ! (GET_CODE (SET_DEST (x)) == SUBREG
 		&& REG_P (SUBREG_REG (SET_DEST (x)))
-		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))
-		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
-		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))
-			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD))))
+		&& !read_modify_subreg_p (SET_DEST (x))))
 	break;
 
       return find_single_use_1 (dest, &SET_SRC (x));
@@ -7275,15 +7272,12 @@ expand_field_assignment (const_rtx x)
 	    }
 	}
 
-      /* A SUBREG between two modes that occupy the same numbers of words
-	 can be done by moving the SUBREG to the source.  */
+      /* If the destination is a subreg that overwrites the whole of the inner
+	 register, we can move the subreg to the source.  */
       else if (GET_CODE (SET_DEST (x)) == SUBREG
 	       /* We need SUBREGs to compute nonzero_bits properly.  */
 	       && nonzero_sign_valid
-	       && (((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))
-		     + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
-		   == ((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))
-			+ (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)))
+	       && !read_modify_subreg_p (SET_DEST (x)))
 	{
 	  x = gen_rtx_SET (SUBREG_REG (SET_DEST (x)),
 			   gen_lowpart
@@ -13821,10 +13815,7 @@ move_deaths (rtx x, rtx maybe_kill_insn,
       if (GET_CODE (dest) == ZERO_EXTRACT
 	  || GET_CODE (dest) == STRICT_LOW_PART
 	  || (GET_CODE (dest) == SUBREG
-	      && (((GET_MODE_SIZE (GET_MODE (dest))
-		    + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
-		  == ((GET_MODE_SIZE (GET_MODE (SUBREG_REG (dest)))
-		       + UNITS_PER_WORD - 1) / UNITS_PER_WORD))))
+	      && !read_modify_subreg_p (dest)))
 	{
 	  move_deaths (dest, maybe_kill_insn, from_luid, to_insn, pnotes);
 	  return;
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	2017-08-24 19:14:37.909164128 +0100
+++ gcc/lra-constraints.c	2017-08-24 19:22:45.218100970 +0100
@@ -1679,7 +1679,7 @@ simplify_operand_subreg (int nop, machin
 	  bitmap_set_bit (&lra_subreg_reload_pseudos, REGNO (new_reg));
 
 	  insert_before = (type != OP_OUT
-			   || GET_MODE_SIZE (innermode) > GET_MODE_SIZE (mode));
+			   || read_modify_subreg_p (operand));
 	  insert_after = (type != OP_IN);
 	  insert_move_for_subreg (insert_before ? &before : NULL,
 				  insert_after ? &after : NULL,
@@ -4242,9 +4242,7 @@ curr_insn_transform (bool check_only_p)
 		     constraints.  */
 		  if (type == OP_OUT
 		      && (curr_static_id->operand[i].strict_low
-			  || (GET_MODE_SIZE (GET_MODE (reg)) > UNITS_PER_WORD
-			      && (GET_MODE_SIZE (mode)
-				  < GET_MODE_SIZE (GET_MODE (reg))))))
+			  || read_modify_subreg_p (*loc)))
 		    type = OP_INOUT;
 		  loc = &SUBREG_REG (*loc);
 		  mode = GET_MODE (*loc);
Index: gcc/lra.c
===================================================================
--- gcc/lra.c	2017-08-24 19:14:37.910164081 +0100
+++ gcc/lra.c	2017-08-24 19:22:45.219101069 +0100
@@ -831,14 +831,12 @@ collect_non_operand_hard_regs (rtx *x, l
   subreg_p = false;
   if (code == SUBREG)
     {
+      if (read_modify_subreg_p (op))
+	subreg_p = true;
       op = SUBREG_REG (op);
       code = GET_CODE (op);
       if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (op)))
-	{
-	  mode = GET_MODE (op);
-	  if (GET_MODE_SIZE (mode) > REGMODE_NATURAL_SIZE (mode))
-	    subreg_p = true;
-	}
+	mode = GET_MODE (op);
     }
   if (REG_P (op))
     {
@@ -1428,14 +1426,12 @@ add_regs_to_insn_regno_info (lra_insn_re
   subreg_p = false;
   if (GET_CODE (x) == SUBREG)
     {
+      if (read_modify_subreg_p (x))
+	subreg_p = true;
       x = SUBREG_REG (x);
       code = GET_CODE (x);
       if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (x)))
-	{
-	  mode = GET_MODE (x);
-	  if (GET_MODE_SIZE (mode) > REGMODE_NATURAL_SIZE (mode))
-	    subreg_p = true;
-	}
+	mode = GET_MODE (x);
     }
   if (REG_P (x))
     {
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2017-08-24 19:22:26.165269825 +0100
+++ gcc/rtlanal.c	2017-08-24 19:22:45.219101069 +0100
@@ -1123,10 +1123,7 @@ reg_referenced_p (const_rtx x, const_rtx
 	  && !REG_P (SET_DEST (body))
 	  && ! (GET_CODE (SET_DEST (body)) == SUBREG
 		&& REG_P (SUBREG_REG (SET_DEST (body)))
-		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (body))))
-		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
-		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (body)))
-			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)))
+		&& !read_modify_subreg_p (SET_DEST (body)))
 	  && reg_overlap_mentioned_p (x, SET_DEST (body)))
 	return 1;
       return 0;
@@ -2015,20 +2012,16 @@ dead_or_set_p (const rtx_insn *insn, con
   return 1;
 }
 
-/* Return TRUE iff DEST is a register or subreg of a register and
-   doesn't change the number of words of the inner register, and any
-   part of the register is TEST_REGNO.  */
+/* Return TRUE iff DEST is a register or subreg of a register, is a
+   complete rather than read-modify-write destination, and contains
+   register TEST_REGNO.  */
 
 static bool
 covers_regno_no_parallel_p (const_rtx dest, unsigned int test_regno)
 {
   unsigned int regno, endregno;
 
-  if (GET_CODE (dest) == SUBREG
-      && (((GET_MODE_SIZE (GET_MODE (dest))
-	    + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
-	  == ((GET_MODE_SIZE (GET_MODE (SUBREG_REG (dest)))
-	       + UNITS_PER_WORD - 1) / UNITS_PER_WORD)))
+  if (GET_CODE (dest) == SUBREG && !read_modify_subreg_p (dest))
     dest = SUBREG_REG (dest);
 
   if (!REG_P (dest))
Jeff Law Oct. 13, 2017, 3:51 p.m. | #3
On 08/24/2017 12:25 PM, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:

>> On Wed, Aug 23, 2017 at 11:49:03AM +0100, Richard Sandiford wrote:

>>> This patch uses df_read_modify_subreg_p to check whether writing

>>> to a subreg would preserve some of the existing contents.

>>

>> combine does not keep the DF info up-to-date -- but that is no

>> problem here, df_read_modify_subreg_p uses no DF info at all.  Maybe

>> it should not have "df_" in the name?

> 

> Yeah, I guess that's a bit confusing.  I've just posted a patch

> to rename it.

> 

> Here's a version of the patch that applies on top of that one.

> Tested as before.  OK to install?

> 

> Thanks,

> Richard

> 

> 

> 2017-08-24  Richard Sandiford  <richard.sandiford@linaro.org>

> 	    Alan Hayward  <alan.hayward@arm.com>

> 	    David Sherwood  <david.sherwood@arm.com>

> 

> gcc/

> 	* caller-save.c (mark_referenced_regs):  Use read_modify_subreg_p.

> 	* combine.c (find_single_use_1): Likewise.

> 	(expand_field_assignment): Likewise.

> 	(move_deaths): Likewise.

> 	* lra-constraints.c (simplify_operand_subreg): Likewise.

> 	(curr_insn_transform): Likewise.

> 	* lra.c (collect_non_operand_hard_regs): Likewise.

> 	(add_regs_to_insn_regno_info): Likewise.

> 	* rtlanal.c (reg_referenced_p): Likewise.

> 	(covers_regno_no_parallel_p): Likewise.

> 



> Index: gcc/combine.c

> ===================================================================

> --- gcc/combine.c	2017-08-24 19:22:26.163269637 +0100

> +++ gcc/combine.c	2017-08-24 19:22:45.218100970 +0100

> @@ -579,10 +579,7 @@ find_single_use_1 (rtx dest, rtx *loc)

>  	  && !REG_P (SET_DEST (x))

>  	  && ! (GET_CODE (SET_DEST (x)) == SUBREG

>  		&& REG_P (SUBREG_REG (SET_DEST (x)))

> -		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))

> -		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)

> -		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))

> -			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD))))

> +		&& !read_modify_subreg_p (SET_DEST (x))))

>  	break;

Is this correct for a paradoxical subreg?  ISTM the original code was
checking for a subreg that just changes the mode, but not the size
(subreg:SI (reg:SF)) or (subreg:DF (reg:DI)) kinds of things.  It would
reject a paradoxical AFAICT.

As written now I think the condition would be true for a paradoxical.

Similarly for the other two instances in combine.c and the changes in
rtlanal.c.

In some of those cases you might be able to argue that it's the right
way to handle a paradoxical.  I haven't thought a whole lot about that
angle, but mention it as a possible way your change might still be correct.


Jeff
Richard Sandiford Oct. 13, 2017, 4:08 p.m. | #4
Jeff Law <law@redhat.com> writes:
> On 08/24/2017 12:25 PM, Richard Sandiford wrote:

>> Segher Boessenkool <segher@kernel.crashing.org> writes:

>>> On Wed, Aug 23, 2017 at 11:49:03AM +0100, Richard Sandiford wrote:

>>>> This patch uses df_read_modify_subreg_p to check whether writing

>>>> to a subreg would preserve some of the existing contents.

>>>

>>> combine does not keep the DF info up-to-date -- but that is no

>>> problem here, df_read_modify_subreg_p uses no DF info at all.  Maybe

>>> it should not have "df_" in the name?

>> 

>> Yeah, I guess that's a bit confusing.  I've just posted a patch

>> to rename it.

>> 

>> Here's a version of the patch that applies on top of that one.

>> Tested as before.  OK to install?

>> 

>> Thanks,

>> Richard

>> 

>> 

>> 2017-08-24  Richard Sandiford  <richard.sandiford@linaro.org>

>> 	    Alan Hayward  <alan.hayward@arm.com>

>> 	    David Sherwood  <david.sherwood@arm.com>

>> 

>> gcc/

>> 	* caller-save.c (mark_referenced_regs):  Use read_modify_subreg_p.

>> 	* combine.c (find_single_use_1): Likewise.

>> 	(expand_field_assignment): Likewise.

>> 	(move_deaths): Likewise.

>> 	* lra-constraints.c (simplify_operand_subreg): Likewise.

>> 	(curr_insn_transform): Likewise.

>> 	* lra.c (collect_non_operand_hard_regs): Likewise.

>> 	(add_regs_to_insn_regno_info): Likewise.

>> 	* rtlanal.c (reg_referenced_p): Likewise.

>> 	(covers_regno_no_parallel_p): Likewise.

>> 

>

>

>> Index: gcc/combine.c

>> ===================================================================

>> --- gcc/combine.c	2017-08-24 19:22:26.163269637 +0100

>> +++ gcc/combine.c	2017-08-24 19:22:45.218100970 +0100

>> @@ -579,10 +579,7 @@ find_single_use_1 (rtx dest, rtx *loc)

>>  	  && !REG_P (SET_DEST (x))

>>  	  && ! (GET_CODE (SET_DEST (x)) == SUBREG

>>  		&& REG_P (SUBREG_REG (SET_DEST (x)))

>> -		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))

>> -		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)

>> -		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))

>> -			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD))))

>> +		&& !read_modify_subreg_p (SET_DEST (x))))

>>  	break;

> Is this correct for a paradoxical subreg?  ISTM the original code was

> checking for a subreg that just changes the mode, but not the size

> (subreg:SI (reg:SF)) or (subreg:DF (reg:DI)) kinds of things.  It would

> reject a paradoxical AFAICT.

>

> As written now I think the condition would be true for a paradoxical.

>

> Similarly for the other two instances in combine.c and the changes in

> rtlanal.c.

>

> In some of those cases you might be able to argue that it's the right

> way to handle a paradoxical.  I haven't thought a whole lot about that

> angle, but mention it as a possible way your change might still be correct.


Yeah, I agree this'll change the handling of paradoxical subregs that
occupy more words than the SUBREG_REG, but I think the new version is
correct.  The comment says:

      /* If the destination is anything other than CC0, PC, a REG or a SUBREG
	 of a REG that occupies all of the REG, the insn uses DEST if
	 it is mentioned in the destination or the source.  Otherwise, we
	 need just check the source.  */

and a paradoxical subreg does occupy all of the SUBREG_REG.

The code is trying to work out whether the instruction "reads" the
destination if you view partial stores as a read of the old value
followed by a write of a partially-updated value, whereas writing to a
paradoxical subreg preserves none of the original value.  And that's
also the semantics that the current code uses for "normal" word-sized
paradoxical subregs.

Thanks,
Richard
Segher Boessenkool Oct. 13, 2017, 8:56 p.m. | #5
On Fri, Oct 13, 2017 at 05:08:51PM +0100, Richard Sandiford wrote:
> Yeah, I agree this'll change the handling of paradoxical subregs that

> occupy more words than the SUBREG_REG, but I think the new version is

> correct.  The comment says:

> 

>       /* If the destination is anything other than CC0, PC, a REG or a SUBREG

> 	 of a REG that occupies all of the REG, the insn uses DEST if

> 	 it is mentioned in the destination or the source.  Otherwise, we

> 	 need just check the source.  */

> 

> and a paradoxical subreg does occupy all of the SUBREG_REG.


I don't think a paradoxical subreg is allowed to refer to more than one
word at all?  At least that is how I read the documentation.  For example
the very first line of the subreg documentation:

@code{subreg} expressions are used to refer to a register in a machine
mode other than its natural one, or to refer to one register of
a multi-part @code{reg} that actually refers to several registers.


Segher
Jeff Law Oct. 23, 2017, 11:16 p.m. | #6
On 10/13/2017 10:08 AM, Richard Sandiford wrote:
> Jeff Law <law@redhat.com> writes:

>> On 08/24/2017 12:25 PM, Richard Sandiford wrote:

>>> Segher Boessenkool <segher@kernel.crashing.org> writes:

>>>> On Wed, Aug 23, 2017 at 11:49:03AM +0100, Richard Sandiford wrote:

>>>>> This patch uses df_read_modify_subreg_p to check whether writing

>>>>> to a subreg would preserve some of the existing contents.

>>>>

>>>> combine does not keep the DF info up-to-date -- but that is no

>>>> problem here, df_read_modify_subreg_p uses no DF info at all.  Maybe

>>>> it should not have "df_" in the name?

>>>

>>> Yeah, I guess that's a bit confusing.  I've just posted a patch

>>> to rename it.

>>>

>>> Here's a version of the patch that applies on top of that one.

>>> Tested as before.  OK to install?

>>>

>>> Thanks,

>>> Richard

>>>

>>>

>>> 2017-08-24  Richard Sandiford  <richard.sandiford@linaro.org>

>>> 	    Alan Hayward  <alan.hayward@arm.com>

>>> 	    David Sherwood  <david.sherwood@arm.com>

>>>

>>> gcc/

>>> 	* caller-save.c (mark_referenced_regs):  Use read_modify_subreg_p.

>>> 	* combine.c (find_single_use_1): Likewise.

>>> 	(expand_field_assignment): Likewise.

>>> 	(move_deaths): Likewise.

>>> 	* lra-constraints.c (simplify_operand_subreg): Likewise.

>>> 	(curr_insn_transform): Likewise.

>>> 	* lra.c (collect_non_operand_hard_regs): Likewise.

>>> 	(add_regs_to_insn_regno_info): Likewise.

>>> 	* rtlanal.c (reg_referenced_p): Likewise.

>>> 	(covers_regno_no_parallel_p): Likewise.

>>>

>>

>>

>>> Index: gcc/combine.c

>>> ===================================================================

>>> --- gcc/combine.c	2017-08-24 19:22:26.163269637 +0100

>>> +++ gcc/combine.c	2017-08-24 19:22:45.218100970 +0100

>>> @@ -579,10 +579,7 @@ find_single_use_1 (rtx dest, rtx *loc)

>>>  	  && !REG_P (SET_DEST (x))

>>>  	  && ! (GET_CODE (SET_DEST (x)) == SUBREG

>>>  		&& REG_P (SUBREG_REG (SET_DEST (x)))

>>> -		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))

>>> -		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)

>>> -		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))

>>> -			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD))))

>>> +		&& !read_modify_subreg_p (SET_DEST (x))))

>>>  	break;

>> Is this correct for a paradoxical subreg?  ISTM the original code was

>> checking for a subreg that just changes the mode, but not the size

>> (subreg:SI (reg:SF)) or (subreg:DF (reg:DI)) kinds of things.  It would

>> reject a paradoxical AFAICT.

>>

>> As written now I think the condition would be true for a paradoxical.

>>

>> Similarly for the other two instances in combine.c and the changes in

>> rtlanal.c.

>>

>> In some of those cases you might be able to argue that it's the right

>> way to handle a paradoxical.  I haven't thought a whole lot about that

>> angle, but mention it as a possible way your change might still be correct.

> 

> Yeah, I agree this'll change the handling of paradoxical subregs that

> occupy more words than the SUBREG_REG, but I think the new version is

> correct.  The comment says:

> 

>       /* If the destination is anything other than CC0, PC, a REG or a SUBREG

> 	 of a REG that occupies all of the REG, the insn uses DEST if

> 	 it is mentioned in the destination or the source.  Otherwise, we

> 	 need just check the source.  */

> 

> and a paradoxical subreg does occupy all of the SUBREG_REG.

> 

> The code is trying to work out whether the instruction "reads" the

> destination if you view partial stores as a read of the old value

> followed by a write of a partially-updated value, whereas writing to a

> paradoxical subreg preserves none of the original value.  And that's

> also the semantics that the current code uses for "normal" word-sized

> paradoxical subregs.

OK.    Thanks for clarifying.

Jeff

Patch

Index: gcc/df.h
===================================================================
--- gcc/df.h	2017-05-18 07:51:11.853804033 +0100
+++ gcc/df.h	2017-08-23 10:44:56.294480753 +0100
@@ -1080,7 +1080,7 @@  extern unsigned int df_hard_reg_used_cou
 extern bool df_regs_ever_live_p (unsigned int);
 extern void df_set_regs_ever_live (unsigned int, bool);
 extern void df_compute_regs_ever_live (bool);
-extern bool df_read_modify_subreg_p (rtx);
+extern bool df_read_modify_subreg_p (const_rtx);
 extern void df_scan_verify (void);
 
 
Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c	2017-08-21 15:49:31.653164829 +0100
+++ gcc/df-scan.c	2017-08-23 10:44:56.294480753 +0100
@@ -2629,7 +2629,7 @@  df_ref_record (enum df_ref_class cl,
    This function returns true iff the SUBREG X is such a SUBREG.  */
 
 bool
-df_read_modify_subreg_p (rtx x)
+df_read_modify_subreg_p (const_rtx x)
 {
   unsigned int isize, osize;
   if (GET_CODE (x) != SUBREG)
Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	2017-08-21 15:49:31.653164829 +0100
+++ gcc/caller-save.c	2017-08-23 10:44:56.292488492 +0100
@@ -1034,10 +1034,7 @@  mark_referenced_regs (rtx *loc, refmarke
 	      /* If we're setting only part of a multi-word register,
 		 we shall mark it as referenced, because the words
 		 that are not being set should be restored.  */
-	      && ((GET_MODE_SIZE (GET_MODE (*loc))
-		   >= GET_MODE_SIZE (GET_MODE (SUBREG_REG (*loc))))
-		  || (GET_MODE_SIZE (GET_MODE (SUBREG_REG (*loc)))
-		      <= UNITS_PER_WORD))))
+	      && !df_read_modify_subreg_p (*loc)))
 	return;
     }
   if (code == MEM || code == SUBREG)
Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2017-08-23 10:44:17.183477418 +0100
+++ gcc/combine.c	2017-08-23 10:44:56.293484623 +0100
@@ -579,10 +579,7 @@  find_single_use_1 (rtx dest, rtx *loc)
 	  && !REG_P (SET_DEST (x))
 	  && ! (GET_CODE (SET_DEST (x)) == SUBREG
 		&& REG_P (SUBREG_REG (SET_DEST (x)))
-		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))
-		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
-		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))
-			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD))))
+		&& !df_read_modify_subreg_p (SET_DEST (x))))
 	break;
 
       return find_single_use_1 (dest, &SET_SRC (x));
@@ -7279,15 +7276,12 @@  expand_field_assignment (const_rtx x)
 	    }
 	}
 
-      /* A SUBREG between two modes that occupy the same numbers of words
-	 can be done by moving the SUBREG to the source.  */
+      /* If the destination is a subreg that overwrites the whole of the inner
+	 register, we can move the subreg to the source.  */
       else if (GET_CODE (SET_DEST (x)) == SUBREG
 	       /* We need SUBREGs to compute nonzero_bits properly.  */
 	       && nonzero_sign_valid
-	       && (((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))
-		     + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
-		   == ((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))
-			+ (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)))
+	       && !df_read_modify_subreg_p (SET_DEST (x)))
 	{
 	  x = gen_rtx_SET (SUBREG_REG (SET_DEST (x)),
 			   gen_lowpart
@@ -13838,10 +13832,7 @@  move_deaths (rtx x, rtx maybe_kill_insn,
       if (GET_CODE (dest) == ZERO_EXTRACT
 	  || GET_CODE (dest) == STRICT_LOW_PART
 	  || (GET_CODE (dest) == SUBREG
-	      && (((GET_MODE_SIZE (GET_MODE (dest))
-		    + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
-		  == ((GET_MODE_SIZE (GET_MODE (SUBREG_REG (dest)))
-		       + UNITS_PER_WORD - 1) / UNITS_PER_WORD))))
+	      && !df_read_modify_subreg_p (dest)))
 	{
 	  move_deaths (dest, maybe_kill_insn, from_luid, to_insn, pnotes);
 	  return;
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	2017-08-22 17:14:30.337866206 +0100
+++ gcc/lra-constraints.c	2017-08-23 10:44:56.295476884 +0100
@@ -1680,7 +1680,7 @@  simplify_operand_subreg (int nop, machin
 	  bitmap_set_bit (&lra_subreg_reload_pseudos, REGNO (new_reg));
 
 	  insert_before = (type != OP_OUT
-			   || GET_MODE_SIZE (innermode) > GET_MODE_SIZE (mode));
+			   || df_read_modify_subreg_p (operand));
 	  insert_after = (type != OP_IN);
 	  insert_move_for_subreg (insert_before ? &before : NULL,
 				  insert_after ? &after : NULL,
@@ -4244,9 +4244,7 @@  curr_insn_transform (bool check_only_p)
 		     constraints.  */
 		  if (type == OP_OUT
 		      && (curr_static_id->operand[i].strict_low
-			  || (GET_MODE_SIZE (GET_MODE (reg)) > UNITS_PER_WORD
-			      && (GET_MODE_SIZE (mode)
-				  < GET_MODE_SIZE (GET_MODE (reg))))))
+			  || df_read_modify_subreg_p (*loc)))
 		    type = OP_INOUT;
 		  loc = &SUBREG_REG (*loc);
 		  mode = GET_MODE (*loc);
Index: gcc/lra.c
===================================================================
--- gcc/lra.c	2017-08-21 15:49:31.652164829 +0100
+++ gcc/lra.c	2017-08-23 10:44:56.295476884 +0100
@@ -831,14 +831,12 @@  collect_non_operand_hard_regs (rtx *x, l
   subreg_p = false;
   if (code == SUBREG)
     {
+      if (df_read_modify_subreg_p (op))
+	subreg_p = true;
       op = SUBREG_REG (op);
       code = GET_CODE (op);
       if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (op)))
-	{
-	  mode = GET_MODE (op);
-	  if (GET_MODE_SIZE (mode) > REGMODE_NATURAL_SIZE (mode))
-	    subreg_p = true;
-	}
+	mode = GET_MODE (op);
     }
   if (REG_P (op))
     {
@@ -1428,14 +1426,12 @@  add_regs_to_insn_regno_info (lra_insn_re
   subreg_p = false;
   if (GET_CODE (x) == SUBREG)
     {
+      if (df_read_modify_subreg_p (x))
+	subreg_p = true;
       x = SUBREG_REG (x);
       code = GET_CODE (x);
       if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (x)))
-	{
-	  mode = GET_MODE (x);
-	  if (GET_MODE_SIZE (mode) > REGMODE_NATURAL_SIZE (mode))
-	    subreg_p = true;
-	}
+	mode = GET_MODE (x);
     }
   if (REG_P (x))
     {
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2017-08-23 10:44:17.187477282 +0100
+++ gcc/rtlanal.c	2017-08-23 10:44:56.296473014 +0100
@@ -1123,10 +1123,7 @@  reg_referenced_p (const_rtx x, const_rtx
 	  && !REG_P (SET_DEST (body))
 	  && ! (GET_CODE (SET_DEST (body)) == SUBREG
 		&& REG_P (SUBREG_REG (SET_DEST (body)))
-		&& (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (body))))
-		      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
-		    == ((GET_MODE_SIZE (GET_MODE (SET_DEST (body)))
-			 + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)))
+		&& !df_read_modify_subreg_p (SET_DEST (body)))
 	  && reg_overlap_mentioned_p (x, SET_DEST (body)))
 	return 1;
       return 0;
@@ -1999,20 +1996,16 @@  dead_or_set_p (const rtx_insn *insn, con
   return 1;
 }
 
-/* Return TRUE iff DEST is a register or subreg of a register and
-   doesn't change the number of words of the inner register, and any
-   part of the register is TEST_REGNO.  */
+/* Return TRUE iff DEST is a register or subreg of a register, is a
+   complete rather than read-modify-write destination, and contains
+   register TEST_REGNO.  */
 
 static bool
 covers_regno_no_parallel_p (const_rtx dest, unsigned int test_regno)
 {
   unsigned int regno, endregno;
 
-  if (GET_CODE (dest) == SUBREG
-      && (((GET_MODE_SIZE (GET_MODE (dest))
-	    + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
-	  == ((GET_MODE_SIZE (GET_MODE (SUBREG_REG (dest)))
-	       + UNITS_PER_WORD - 1) / UNITS_PER_WORD)))
+  if (GET_CODE (dest) == SUBREG && !df_read_modify_subreg_p (dest))
     dest = SUBREG_REG (dest);
 
   if (!REG_P (dest))