Re: Add subreg_memory_offset helper functions

[Jeff Law]

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  
>   Alan Hayward  
>   David Sherwood  
> 
> 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

Add subreg_memory_offset helper functions

[Richard Sandiford]

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  
Alan Hayward  
David Sherwood  

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.

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.c2017-08-27 23:19:56.284397205 +0100
+++ gcc/expmed.c2017-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
-