Add subreg_memory_offset helper functions

Message ID 87shgct84a.fsf@linaro.org
State New
Headers show
Series
  • Add subreg_memory_offset helper functions
Related show

Commit Message

Richard Sandiford Aug. 28, 2017, 8:21 a.m.
This patch adds routines for converting a SUBREG_BYTE offset into a
memory address offset.  The two only differ for paradoxical subregs,
where SUBREG_BYTE is always 0 but the memory address offset can be
negative.

Tested on aarch64-linux-gnu and x86_64-linux-gnu, and by checking
there was no change in the testsuite assembly output for at least
one target per CPU.  OK to install?

Richard


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

gcc/
	* rtl.h (subreg_memory_offset): Declare.
	* emit-rtl.c (subreg_memory_offset): New function.
	* expmed.c (store_bit_field_1): Use it.
	* expr.c (undefined_operand_subword_p): Likewise.
	* simplify-rtx.c (simplify_subreg): Likewise.

Comments

Jeff Law Sept. 4, 2017, 5:15 a.m. | #1
On 08/28/2017 02:21 AM, Richard Sandiford wrote:
> This patch adds routines for converting a SUBREG_BYTE offset into a

> memory address offset.  The two only differ for paradoxical subregs,

> where SUBREG_BYTE is always 0 but the memory address offset can be

> negative.

> 

> Tested on aarch64-linux-gnu and x86_64-linux-gnu, and by checking

> there was no change in the testsuite assembly output for at least

> one target per CPU.  OK to install?

> 

> Richard

> 

> 

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

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

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

> 

> gcc/

> 	* rtl.h (subreg_memory_offset): Declare.

> 	* emit-rtl.c (subreg_memory_offset): New function.

> 	* expmed.c (store_bit_field_1): Use it.

> 	* expr.c (undefined_operand_subword_p): Likewise.

> 	* simplify-rtx.c (simplify_subreg): Likewise.

I hate reading though the subreg related stuff.  It just seems far too
easy to get wrong.  So while I applaud the refactoring, I hate doing the
review on it ;(

Anyway, this seems reasonable.  I didn't follow the mixed endianness
case as thoroughly as the others.  But I trust your judgment & testing.

jeff

Patch

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2017-08-27 23:19:56.284397205 +0100
+++ gcc/rtl.h	2017-08-28 09:19:13.737714836 +0100
@@ -2827,6 +2827,8 @@  subreg_highpart_offset (machine_mode out
 }
 
 extern int byte_lowpart_offset (machine_mode, machine_mode);
+extern int subreg_memory_offset (machine_mode, machine_mode, unsigned int);
+extern int subreg_memory_offset (const_rtx);
 extern rtx make_safe_from (rtx, rtx);
 extern rtx convert_memory_address_addr_space_1 (machine_mode, rtx,
 						addr_space_t, bool, bool);
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2017-08-22 17:14:30.335896832 +0100
+++ gcc/emit-rtl.c	2017-08-28 09:19:13.736814836 +0100
@@ -1013,6 +1013,33 @@  byte_lowpart_offset (machine_mode outer_
   else
     return subreg_lowpart_offset (outer_mode, inner_mode);
 }
+
+/* Return the offset of (subreg:OUTER_MODE (mem:INNER_MODE X) OFFSET)
+   from address X.  For paradoxical big-endian subregs this is a
+   negative value, otherwise it's the same as OFFSET.  */
+
+int
+subreg_memory_offset (machine_mode outer_mode, machine_mode inner_mode,
+		      unsigned int offset)
+{
+  if (paradoxical_subreg_p (outer_mode, inner_mode))
+    {
+      gcc_assert (offset == 0);
+      return -subreg_lowpart_offset (inner_mode, outer_mode);
+    }
+  return offset;
+}
+
+/* As above, but return the offset that existing subreg X would have
+   if SUBREG_REG (X) were stored in memory.  The only significant thing
+   about the current SUBREG_REG is its mode.  */
+
+int
+subreg_memory_offset (const_rtx x)
+{
+  return subreg_memory_offset (GET_MODE (x), GET_MODE (SUBREG_REG (x)),
+			       SUBREG_BYTE (x));
+}
 
 /* Generate a REG rtx for a new pseudo register of mode MODE.
    This pseudo is assigned the next sequential register number.  */
Index: gcc/expmed.c
===================================================================
--- gcc/expmed.c	2017-08-27 23:19:56.284397205 +0100
+++ gcc/expmed.c	2017-08-28 09:19:13.736814836 +0100
@@ -726,29 +726,7 @@  store_bit_field_1 (rtx str_rtx, unsigned
 
   while (GET_CODE (op0) == SUBREG)
     {
-      /* The following line once was done only if WORDS_BIG_ENDIAN,
-	 but I think that is a mistake.  WORDS_BIG_ENDIAN is
-	 meaningful at a much higher level; when structures are copied
-	 between memory and regs, the higher-numbered regs
-	 always get higher addresses.  */
-      int inner_mode_size = GET_MODE_SIZE (GET_MODE (SUBREG_REG (op0)));
-      int outer_mode_size = GET_MODE_SIZE (GET_MODE (op0));
-      int byte_offset = 0;
-
-      /* Paradoxical subregs need special handling on big-endian machines.  */
-      if (paradoxical_subreg_p (op0))
-	{
-	  int difference = inner_mode_size - outer_mode_size;
-
-	  if (WORDS_BIG_ENDIAN)
-	    byte_offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-	  if (BYTES_BIG_ENDIAN)
-	    byte_offset += difference % UNITS_PER_WORD;
-	}
-      else
-	byte_offset = SUBREG_BYTE (op0);
-
-      bitnum += byte_offset * BITS_PER_UNIT;
+      bitnum += subreg_memory_offset (op0) * BITS_PER_UNIT;
       op0 = SUBREG_REG (op0);
     }
 
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	2017-08-27 23:19:56.149397206 +0100
+++ gcc/expr.c	2017-08-28 09:19:13.737714836 +0100
@@ -3524,30 +3524,12 @@  emit_move_ccmode (machine_mode mode, rtx
 static bool
 undefined_operand_subword_p (const_rtx op, int i)
 {
-  machine_mode innermode, innermostmode;
-  int offset;
   if (GET_CODE (op) != SUBREG)
     return false;
-  innermode = GET_MODE (op);
-  innermostmode = GET_MODE (SUBREG_REG (op));
-  offset = i * UNITS_PER_WORD + SUBREG_BYTE (op);
-  /* The SUBREG_BYTE represents offset, as if the value were stored in
-     memory, except for a paradoxical subreg where we define
-     SUBREG_BYTE to be 0; undo this exception as in
-     simplify_subreg.  */
-  if (SUBREG_BYTE (op) == 0
-      && GET_MODE_SIZE (innermostmode) < GET_MODE_SIZE (innermode))
-    {
-      int difference = (GET_MODE_SIZE (innermostmode) - GET_MODE_SIZE (innermode));
-      if (WORDS_BIG_ENDIAN)
-	offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-      if (BYTES_BIG_ENDIAN)
-	offset += difference % UNITS_PER_WORD;
-    }
-  if (offset >= GET_MODE_SIZE (innermostmode)
-      || offset <= -GET_MODE_SIZE (word_mode))
-    return true;
-  return false;
+  machine_mode innermostmode = GET_MODE (SUBREG_REG (op));
+  HOST_WIDE_INT offset = i * UNITS_PER_WORD + subreg_memory_offset (op);
+  return (offset >= GET_MODE_SIZE (innermostmode)
+	  || offset <= -UNITS_PER_WORD);
 }
 
 /* A subroutine of emit_move_insn_1.  Generate a move from Y into X.
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2017-08-27 23:19:56.284397205 +0100
+++ gcc/simplify-rtx.c	2017-08-28 09:19:13.739514836 +0100
@@ -6050,34 +6050,18 @@  simplify_subreg (machine_mode outermode,
   if (GET_CODE (op) == SUBREG)
     {
       machine_mode innermostmode = GET_MODE (SUBREG_REG (op));
-      int final_offset = byte + SUBREG_BYTE (op);
       rtx newx;
 
       if (outermode == innermostmode
 	  && byte == 0 && SUBREG_BYTE (op) == 0)
 	return SUBREG_REG (op);
 
-      /* The SUBREG_BYTE represents offset, as if the value were stored
-	 in memory.  Irritating exception is paradoxical subreg, where
-	 we define SUBREG_BYTE to be 0.  On big endian machines, this
-	 value should be negative.  For a moment, undo this exception.  */
-      if (byte == 0 && GET_MODE_SIZE (innermode) < GET_MODE_SIZE (outermode))
-	{
-	  int difference = (GET_MODE_SIZE (innermode) - GET_MODE_SIZE (outermode));
-	  if (WORDS_BIG_ENDIAN)
-	    final_offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-	  if (BYTES_BIG_ENDIAN)
-	    final_offset += difference % UNITS_PER_WORD;
-	}
-      if (SUBREG_BYTE (op) == 0
-	  && GET_MODE_SIZE (innermostmode) < GET_MODE_SIZE (innermode))
-	{
-	  int difference = (GET_MODE_SIZE (innermostmode) - GET_MODE_SIZE (innermode));
-	  if (WORDS_BIG_ENDIAN)
-	    final_offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-	  if (BYTES_BIG_ENDIAN)
-	    final_offset += difference % UNITS_PER_WORD;
-	}
+      /* Work out the memory offset of the final OUTERMODE value relative
+	 to the inner value of OP.  */
+      HOST_WIDE_INT mem_offset = subreg_memory_offset (outermode,
+						       innermode, byte);
+      HOST_WIDE_INT op_mem_offset = subreg_memory_offset (op);
+      HOST_WIDE_INT final_offset = mem_offset + op_mem_offset;
 
       /* See whether resulting subreg will be paradoxical.  */
       if (!paradoxical_subreg_p (outermode, innermostmode))
@@ -6092,19 +6076,12 @@  simplify_subreg (machine_mode outermode,
 	}
       else
 	{
-	  int offset = 0;
-	  int difference = (GET_MODE_SIZE (innermostmode) - GET_MODE_SIZE (outermode));
-
-	  /* In paradoxical subreg, see if we are still looking on lower part.
-	     If so, our SUBREG_BYTE will be 0.  */
-	  if (WORDS_BIG_ENDIAN)
-	    offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-	  if (BYTES_BIG_ENDIAN)
-	    offset += difference % UNITS_PER_WORD;
-	  if (offset == final_offset)
-	    final_offset = 0;
-	  else
+	  HOST_WIDE_INT required_offset
+	    = subreg_memory_offset (outermode, innermostmode, 0);
+	  if (final_offset != required_offset)
 	    return NULL_RTX;
+	  /* Paradoxical subregs always have byte offset 0.  */
+	  final_offset = 0;
 	}
 
       /* Recurse for further possible simplifications.  */
@@ -6145,22 +6122,9 @@  simplify_subreg (machine_mode outermode,
       final_regno = simplify_subreg_regno (regno, innermode, byte, outermode);
       if (HARD_REGISTER_NUM_P (final_regno))
 	{
-	  rtx x;
-	  int final_offset = byte;
-
-	  /* Adjust offset for paradoxical subregs.  */
-	  if (byte == 0
-	      && GET_MODE_SIZE (innermode) < GET_MODE_SIZE (outermode))
-	    {
-	      int difference = (GET_MODE_SIZE (innermode)
-				- GET_MODE_SIZE (outermode));
-	      if (WORDS_BIG_ENDIAN)
-		final_offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-	      if (BYTES_BIG_ENDIAN)
-		final_offset += difference % UNITS_PER_WORD;
-	    }
-
-	  x = gen_rtx_REG_offset (op, outermode, final_regno, final_offset);
+	  rtx x = gen_rtx_REG_offset (op, outermode, final_regno,
+				      subreg_memory_offset (outermode,
+							    innermode, byte));
 
 	  /* Propagate original regno.  We don't have any way to specify
 	     the offset inside original regno, so do so only for lowpart.