diff mbox

Add more subreg offset helpers

Message ID 87shqs1z52.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford Nov. 15, 2016, 4:33 p.m. UTC 
Provide versions of subreg_lowpart_offset and subreg_highpart_offset
that work on mode sizes rather than modes.  Also provide a routine
that converts an lsb position to a subreg offset.

The intent (in combination with later patches) is to move the
handling of the BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN case into
just two places, so that for other combinations we don't have
to split offsets into words and subwords.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Thanks,
Richard


[ This patch is part of the SVE series posted here:
  https://gcc.gnu.org/ml/gcc/2016-11/msg00030.html ]

gcc/
2016-11-15  Richard Sandiford  <richard.sandiford@arm.com>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

	* rtl.h (subreg_size_offset_from_lsb): Declare.
	(subreg_offset_from_lsb): Likewise.
	(subreg_size_lowpart_offset): Likewise.
	(subreg_size_highpart_offset): Likewise.
	* emit-rtl.c (subreg_size_lowpart_offset): New function.
	(subreg_lowpart_offset): Use it.
	(subreg_size_highpart_offset): New function.
	(subreg_highpart_offset): Use it.
	* rtlanal.c (subreg_size_offset_from_lsb): New function.
	(subreg_offset_from_lsb): Likewise.

Comments

Eric Botcazou Nov. 16, 2016, 8:03 a.m. UTC | #1 
> Provide versions of subreg_lowpart_offset and subreg_highpart_offset
> that work on mode sizes rather than modes.  Also provide a routine
> that converts an lsb position to a subreg offset.

The position is in bits but all the other parameters and the return value are 
in bytes so IMO this is a recipe for problems.  I'd also rename LSB into DIFF 
or LSB_OFFSET or something equivalent because LSB is not very explicit.

> 2016-11-15  Richard Sandiford  <richard.sandiford@arm.com>
> 	    Alan Hayward  <alan.hayward@arm.com>
> 	    David Sherwood  <david.sherwood@arm.com>
> 
> 	* rtl.h (subreg_size_offset_from_lsb): Declare.
> 	(subreg_offset_from_lsb): Likewise.
> 	(subreg_size_lowpart_offset): Likewise.
> 	(subreg_size_highpart_offset): Likewise.
> 	* emit-rtl.c (subreg_size_lowpart_offset): New function.
> 	(subreg_lowpart_offset): Use it.
> 	(subreg_size_highpart_offset): New function.
> 	(subreg_highpart_offset): Use it.
> 	* rtlanal.c (subreg_size_offset_from_lsb): New function.
> 	(subreg_offset_from_lsb): Likewise.

Please make the 3 wrappers inline functions.
Richard Sandiford Nov. 16, 2016, 9:55 a.m. UTC | #2 
Eric Botcazou <ebotcazou@adacore.com> writes:
>> Provide versions of subreg_lowpart_offset and subreg_highpart_offset
>> that work on mode sizes rather than modes.  Also provide a routine
>> that converts an lsb position to a subreg offset.
>
> The position is in bits but all the other parameters and the return
> value are in bytes so IMO this is a recipe for problems.  I'd also
> rename LSB into DIFF or LSB_OFFSET or something equivalent because LSB
> is not very explicit.

subreg_offset_from_lsb was supposed to be the inverse operation of
subreg_lsb, which also returns a bit number.  Should I change that
to return a byte number as well?

Thanks,
Richard
Eric Botcazou Nov. 16, 2016, 10:26 a.m. UTC | #3 
> subreg_offset_from_lsb was supposed to be the inverse operation of
> subreg_lsb, which also returns a bit number.

It would have helped the reviewer to state it in the function comment. ;-)

> Should I change that to return a byte number as well?

Both functions are fine as-is, but mention that the new one is the inverse of 
the old one in the comment.
diff mbox

Patch

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 9ea0c8f..bc4e536 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -1478,44 +1478,59 @@  gen_highpart_mode (machine_mode outermode, machine_mode innermode, rtx exp)
 			      subreg_highpart_offset (outermode, innermode));
 }
 
-/* Return the SUBREG_BYTE for an OUTERMODE lowpart of an INNERMODE value.  */
+/* Return the SUBREG_BYTE for a lowpart subreg whose outer mode has
+   OUTER_BYTES bytes and whose inner mode has INNER_BYTES bytes.  */
 
 unsigned int
-subreg_lowpart_offset (machine_mode outermode, machine_mode innermode)
+subreg_size_lowpart_offset (unsigned int outer_bytes, unsigned int inner_bytes)
 {
-  unsigned int offset = 0;
-  int difference = (GET_MODE_SIZE (innermode) - GET_MODE_SIZE (outermode));
+  if (outer_bytes > inner_bytes)
+    /* Paradoxical subregs always have a SUBREG_BYTE of 0.  */
+    return 0;
 
-  if (difference > 0)
-    {
-      if (WORDS_BIG_ENDIAN)
-	offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-      if (BYTES_BIG_ENDIAN)
-	offset += difference % UNITS_PER_WORD;
-    }
+  if (BYTES_BIG_ENDIAN && WORDS_BIG_ENDIAN)
+    return inner_bytes - outer_bytes;
+  else if (!BYTES_BIG_ENDIAN && !WORDS_BIG_ENDIAN)
+    return 0;
+  else
+    return subreg_size_offset_from_lsb (outer_bytes, inner_bytes, 0);
+}
+
+/* Return the SUBREG_BYTE for an OUTERMODE lowpart of an INNERMODE value.  */
 
-  return offset;
+unsigned int
+subreg_lowpart_offset (machine_mode outermode, machine_mode innermode)
+{
+  return subreg_size_lowpart_offset (GET_MODE_SIZE (outermode),
+				     GET_MODE_SIZE (innermode));
 }
 
-/* Return offset in bytes to get OUTERMODE high part
-   of the value in mode INNERMODE stored in memory in target format.  */
+/* Return the SUBREG_BYTE for a highpart subreg whose outer mode has
+   OUTER_BYTES bytes and whose inner mode has INNER_BYTES bytes.  */
+
 unsigned int
-subreg_highpart_offset (machine_mode outermode, machine_mode innermode)
+subreg_size_highpart_offset (unsigned int outer_bytes,
+			     unsigned int inner_bytes)
 {
-  unsigned int offset = 0;
-  int difference = (GET_MODE_SIZE (innermode) - GET_MODE_SIZE (outermode));
+  gcc_assert (inner_bytes >= outer_bytes);
 
-  gcc_assert (GET_MODE_SIZE (innermode) >= GET_MODE_SIZE (outermode));
+  if (BYTES_BIG_ENDIAN && WORDS_BIG_ENDIAN)
+    return 0;
+  else if (!BYTES_BIG_ENDIAN && !WORDS_BIG_ENDIAN)
+    return inner_bytes - outer_bytes;
+  else
+    return subreg_size_offset_from_lsb (outer_bytes, inner_bytes,
+					(inner_bytes - outer_bytes)
+					* BITS_PER_UNIT);
+}
 
-  if (difference > 0)
-    {
-      if (! WORDS_BIG_ENDIAN)
-	offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
-      if (! BYTES_BIG_ENDIAN)
-	offset += difference % UNITS_PER_WORD;
-    }
+/* Return the SUBREG_BYTE for an OUTERMODE highpart of an INNERMODE value.  */
 
-  return offset;
+unsigned int
+subreg_highpart_offset (machine_mode outermode, machine_mode innermode)
+{
+  return subreg_size_highpart_offset (GET_MODE_SIZE (outermode),
+				      GET_MODE_SIZE (innermode));
 }
 
 /* Return 1 iff X, assumed to be a SUBREG,
diff --git a/gcc/rtl.h b/gcc/rtl.h
index df5172b..2fca974 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2178,6 +2178,10 @@  extern void get_full_rtx_cost (rtx, machine_mode, enum rtx_code, int,
 extern unsigned int subreg_lsb (const_rtx);
 extern unsigned int subreg_lsb_1 (machine_mode, machine_mode,
 				  unsigned int);
+extern unsigned int subreg_size_offset_from_lsb (unsigned int, unsigned int,
+						 unsigned int);
+extern unsigned int subreg_offset_from_lsb (machine_mode, machine_mode,
+					    unsigned int);
 extern unsigned int subreg_regno_offset	(unsigned int, machine_mode,
 					 unsigned int, machine_mode);
 extern bool subreg_offset_representable_p (unsigned int, machine_mode,
@@ -2764,8 +2768,10 @@  extern rtx operand_subword (rtx, unsigned int, int, machine_mode);
 extern rtx operand_subword_force (rtx, unsigned int, machine_mode);
 extern bool paradoxical_subreg_p (const_rtx);
 extern int subreg_lowpart_p (const_rtx);
+extern unsigned int subreg_size_lowpart_offset (unsigned int, unsigned int);
 extern unsigned int subreg_lowpart_offset (machine_mode,
 					   machine_mode);
+extern unsigned int subreg_size_highpart_offset (unsigned int, unsigned int);
 extern unsigned int subreg_highpart_offset (machine_mode,
 					    machine_mode);
 extern int byte_lowpart_offset (machine_mode, machine_mode);
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 889b14d..ca6cced 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -3528,6 +3528,55 @@  subreg_lsb (const_rtx x)
 		       SUBREG_BYTE (x));
 }
 
+/* Return the subreg byte offset for a subreg whose outer value has
+   OUTER_BYTES bytes, whose inner value has INNER_BYTES bytes,
+   and where there are LSB bits between the lsb of the outer value
+   and the lsb of the inner value.  */
+
+unsigned int
+subreg_size_offset_from_lsb (unsigned int outer_bytes,
+			     unsigned int inner_bytes,
+			     unsigned int lsb)
+{
+  /* A paradoxical subreg begins at bit position 0.  */
+  if (outer_bytes > inner_bytes)
+    {
+      gcc_checking_assert (lsb == 0);
+      return 0;
+    }
+
+  gcc_assert (lsb % BITS_PER_UNIT == 0);
+  unsigned int lower_bytes = lsb / BITS_PER_UNIT;
+  unsigned int upper_bytes = inner_bytes - (lower_bytes + outer_bytes);
+  if (WORDS_BIG_ENDIAN && BYTES_BIG_ENDIAN)
+    return upper_bytes;
+  else if (!WORDS_BIG_ENDIAN && !BYTES_BIG_ENDIAN)
+    return lower_bytes;
+  else
+    {
+      unsigned int lower_word_part = lower_bytes & -UNITS_PER_WORD;
+      unsigned int upper_word_part = upper_bytes & -UNITS_PER_WORD;
+      if (WORDS_BIG_ENDIAN)
+	return upper_word_part + (lower_bytes - lower_word_part);
+      else
+	return lower_word_part + (upper_bytes - upper_word_part);
+    }
+}
+
+/* Return the subreg byte offset for a subreg whose outer mode is
+   OUTER_MODE, whose inner mode is INNER_MODE, and where there are
+   LSB bits between the lsb of the outer value and the lsb of the
+   inner value.  This is the inverse of subreg_lsb_1.  */
+
+unsigned int
+subreg_offset_from_lsb (machine_mode outer_mode,
+			machine_mode inner_mode,
+			unsigned int lsb)
+{
+  return subreg_size_offset_from_lsb (GET_MODE_SIZE (outer_mode),
+				      GET_MODE_SIZE (inner_mode), lsb);
+}
+
 /* Fill in information about a subreg of a hard register.
    xregno - A regno of an inner hard subreg_reg (or what will become one).
    xmode  - The mode of xregno.