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

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