diff mbox series

, V4, patch #9 [part of patch #4.2], Add prefixed address offset checks

Message ID 20191004122911.GA561@ibm-tinman.the-meissners.org
State New
Headers show
Series , V4, patch #9 [part of patch #4.2], Add prefixed address offset checks | expand

Commit Message

Michael Meissner Oct. 4, 2019, 12:29 p.m. UTC 
I was asked to split V4 patch #4.2 into smaller chuncks.  This patch is one of
8 patches that were broken out from 4.2.  Another patch from 4.2 to use
SIGNED_16BIT_OFFSET_EXTRA_P has already been committed.

This patch adds checks in the various places that check whether an offset is
valid to allow numeric 34-bit offsets and PC-relative offsets that prefixed
memory instructions adds.

Using all of the patches in this series, I have bootstrapped the compiler on a
little endian power8 system and ran the regression tests.  In addition, I have
built the Spec 2006 and 2017 benchmark suites, for -mcpu=power8, -mcpu=power9,
and -mcpu=future, and all of the benchmarks build.  Can I check this into the
trunk?

2019-10-03  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.c (quad_address_p): Add check for prefixed
	addresses.
	(mem_operand_gpr): Add check for prefixed addresses.
	(mem_operand_ds_form): Add check for prefixed addresses.
	(rs6000_legitimate_offset_address_p): If we support prefixed
	addresses, check for a 34-bit offset instead of 16-bit.
	(rs6000_legitimate_address_p): Add check for prefixed addresses.
	Do not allow load/store with update if the address is prefixed.
	(rs6000_mode_dependent_address):  If we support prefixed
	addresses, check for a 34-bit offset instead of 16-bit.

Comments

Segher Boessenkool Oct. 9, 2019, 9:56 p.m. UTC | #1 
Hi!

On Fri, Oct 04, 2019 at 08:29:11AM -0400, Michael Meissner wrote:
> @@ -8651,6 +8675,11 @@ rs6000_legitimate_address_p (machine_mod
>        && mode_supports_pre_incdec_p (mode)
>        && legitimate_indirect_address_p (XEXP (x, 0), reg_ok_strict))
>      return 1;
> +
> +  /* Handle prefixed addresses (PC-relative or 34-bit offset).  */
> +  if (address_is_prefixed (x, mode, NON_PREFIXED_DEFAULT))
> +    return 1;

Is this correct?  Are addresses with a larger offset always legitimate?
I don't see why that would be the case.

The rest of the patch looks good, thanks.


Segher
Michael Meissner Oct. 9, 2019, 11:40 p.m. UTC | #2 
On Wed, Oct 09, 2019 at 04:56:48PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Oct 04, 2019 at 08:29:11AM -0400, Michael Meissner wrote:
> > @@ -8651,6 +8675,11 @@ rs6000_legitimate_address_p (machine_mod
> >        && mode_supports_pre_incdec_p (mode)
> >        && legitimate_indirect_address_p (XEXP (x, 0), reg_ok_strict))
> >      return 1;
> > +
> > +  /* Handle prefixed addresses (PC-relative or 34-bit offset).  */
> > +  if (address_is_prefixed (x, mode, NON_PREFIXED_DEFAULT))
> > +    return 1;
> 
> Is this correct?  Are addresses with a larger offset always legitimate?
> I don't see why that would be the case.
> 
> The rest of the patch looks good, thanks.

As far as I know, with the exception of SDmode (which is not allowed to have an
offset) all other modes that use D*-form addresses would work with a prefixed
instruction, assuming the offset fits in the 34-bit field.

The function address_to_insn_form, which is called by address_is_prefixed,
checks if the offset is 34-bits or less, whether the mode is SDmode, etc. are
all valid.
Michael Meissner Oct. 9, 2019, 11:41 p.m. UTC | #3 
On Wed, Oct 09, 2019 at 04:56:48PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Oct 04, 2019 at 08:29:11AM -0400, Michael Meissner wrote:
> > @@ -8651,6 +8675,11 @@ rs6000_legitimate_address_p (machine_mod
> >        && mode_supports_pre_incdec_p (mode)
> >        && legitimate_indirect_address_p (XEXP (x, 0), reg_ok_strict))
> >      return 1;
> > +
> > +  /* Handle prefixed addresses (PC-relative or 34-bit offset).  */
> > +  if (address_is_prefixed (x, mode, NON_PREFIXED_DEFAULT))
> > +    return 1;
> 
> Is this correct?  Are addresses with a larger offset always legitimate?
> I don't see why that would be the case.
> 
> The rest of the patch looks good, thanks.

This patch BTW is the same as the new V5 patch #1.
Segher Boessenkool Oct. 10, 2019, 9:52 p.m. UTC | #4 
On Wed, Oct 09, 2019 at 07:40:23PM -0400, Michael Meissner wrote:
> On Wed, Oct 09, 2019 at 04:56:48PM -0500, Segher Boessenkool wrote:
> > On Fri, Oct 04, 2019 at 08:29:11AM -0400, Michael Meissner wrote:
> > > @@ -8651,6 +8675,11 @@ rs6000_legitimate_address_p (machine_mod
> > >        && mode_supports_pre_incdec_p (mode)
> > >        && legitimate_indirect_address_p (XEXP (x, 0), reg_ok_strict))
> > >      return 1;
> > > +
> > > +  /* Handle prefixed addresses (PC-relative or 34-bit offset).  */
> > > +  if (address_is_prefixed (x, mode, NON_PREFIXED_DEFAULT))
> > > +    return 1;
> > 
> > Is this correct?  Are addresses with a larger offset always legitimate?
> > I don't see why that would be the case.

> The function address_to_insn_form, which is called by address_is_prefixed,
> checks if the offset is 34-bits or less

Ah, right.

And "address_is_prefixed" is a long enough name, "address_is_a_valid_prefixed_address"
isn't better ;-)


Segher
diff mbox series

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 276523)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -7250,6 +7250,13 @@  quad_address_p (rtx addr, machine_mode m
   if (VECTOR_MODE_P (mode) && !mode_supports_dq_form (mode))
     return false;
 
+  /* Is this a valid prefixed address?  If the bottom four bits of the offset
+     are non-zero, we could use a prefixed instruction (which does not have the
+     DQ-form constraint that the traditional instruction had) instead of
+     forcing the unaligned offset to a GPR.  */
+  if (address_is_prefixed (addr, mode, NON_PREFIXED_DQ))
+    return true;
+
   if (GET_CODE (addr) != PLUS)
     return false;
 
@@ -7351,6 +7358,13 @@  mem_operand_gpr (rtx op, machine_mode mo
       && legitimate_indirect_address_p (XEXP (addr, 0), false))
     return true;
 
+  /* Allow prefixed instructions if supported.  If the bottom two bits of the
+     offset are non-zero, we could use a prefixed instruction (which does not
+     have the DS-form constraint that the traditional instruction had) instead
+     of forcing the unaligned offset to a GPR.  */
+  if (address_is_prefixed (addr, mode, NON_PREFIXED_DS))
+    return true;
+
   /* Don't allow non-offsettable addresses.  See PRs 83969 and 84279.  */
   if (!rs6000_offsettable_memref_p (op, mode, false))
     return false;
@@ -7385,6 +7399,13 @@  mem_operand_ds_form (rtx op, machine_mod
   int extra;
   rtx addr = XEXP (op, 0);
 
+  /* Allow prefixed instructions if supported.  If the bottom two bits of the
+     offset are non-zero, we could use a prefixed instruction (which does not
+     have the DS-form constraint that the traditional instruction had) instead
+     of forcing the unaligned offset to a GPR.  */
+  if (address_is_prefixed (addr, mode, NON_PREFIXED_DS))
+    return true;
+
   if (!offsettable_address_p (false, mode, addr))
     return false;
 
@@ -7754,7 +7775,10 @@  rs6000_legitimate_offset_address_p (mach
       break;
     }
 
-  return SIGNED_16BIT_OFFSET_EXTRA_P (offset, extra);
+  if (TARGET_PREFIXED_ADDR)
+    return SIGNED_34BIT_OFFSET_EXTRA_P (offset, extra);
+  else
+    return SIGNED_16BIT_OFFSET_EXTRA_P (offset, extra);
 }
 
 bool
@@ -8651,6 +8675,11 @@  rs6000_legitimate_address_p (machine_mod
       && mode_supports_pre_incdec_p (mode)
       && legitimate_indirect_address_p (XEXP (x, 0), reg_ok_strict))
     return 1;
+
+  /* Handle prefixed addresses (PC-relative or 34-bit offset).  */
+  if (address_is_prefixed (x, mode, NON_PREFIXED_DEFAULT))
+    return 1;
+
   /* Handle restricted vector d-form offsets in ISA 3.0.  */
   if (quad_offset_p)
     {
@@ -8709,7 +8738,11 @@  rs6000_legitimate_address_p (machine_mod
 	  || (!avoiding_indexed_address_p (mode)
 	      && legitimate_indexed_address_p (XEXP (x, 1), reg_ok_strict)))
       && rtx_equal_p (XEXP (XEXP (x, 1), 0), XEXP (x, 0)))
-    return 1;
+    {
+      /* There is no prefixed version of the load/store with update.  */
+      rtx addr = XEXP (x, 1);
+      return !address_is_prefixed (addr, mode, NON_PREFIXED_DEFAULT);
+    }
   if (reg_offset_p && !quad_offset_p
       && legitimate_lo_sum_address_p (mode, x, reg_ok_strict))
     return 1;
@@ -8773,7 +8806,10 @@  rs6000_mode_dependent_address (const_rtx
 	{
 	  HOST_WIDE_INT val = INTVAL (XEXP (addr, 1));
 	  HOST_WIDE_INT extra = TARGET_POWERPC64 ? 8 : 12;
-	  return !SIGNED_16BIT_OFFSET_EXTRA_P (val, extra);
+	  if (TARGET_PREFIXED_ADDR)
+	    return !SIGNED_34BIT_OFFSET_EXTRA_P (val, extra);
+	  else
+	    return !SIGNED_16BIT_OFFSET_EXTRA_P (val, extra);
 	}
       break;