Patchwork [RS6000] PR54009 again

login
register
mail settings
Submitter Alan Modra
Date Feb. 6, 2013, 1:38 p.m.
Message ID <20130206133830.GR5023@bubble.grove.modra.org>
Download mbox | patch
Permalink /patch/218608/
State New
Headers show

Comments

Alan Modra - Feb. 6, 2013, 1:38 p.m.
My PR54131 fix caused PR54009 to raise its ugly head again.  Allowing
all LO_SUM addresses in the 'Y' constraint (mem_operand_gpr) wasn't
quite correct.  We still need to check for wrap when offsetting for
multiple words/regs.  Fixing that means the LO_SUM addresses not
accepted by 'Y' need an address reload.

OK assuming powerpc64 and powerpc-linux regression testing passes?

	PR target/54009
	* config/rs6000/rs6000.c (mem_operand_gpr): Check that LO_SUM
	addresses won't wrap when offsetting.
	(rs6000_secondary_reload): Provide secondary reloads needed for
	wrapping LO_SUM addresses.
David Edelsohn - Feb. 6, 2013, 6:24 p.m.
On Wed, Feb 6, 2013 at 8:38 AM, Alan Modra <amodra@gmail.com> wrote:
> My PR54131 fix caused PR54009 to raise its ugly head again.  Allowing
> all LO_SUM addresses in the 'Y' constraint (mem_operand_gpr) wasn't
> quite correct.  We still need to check for wrap when offsetting for
> multiple words/regs.  Fixing that means the LO_SUM addresses not
> accepted by 'Y' need an address reload.
>
> OK assuming powerpc64 and powerpc-linux regression testing passes?
>
>         PR target/54009
>         * config/rs6000/rs6000.c (mem_operand_gpr): Check that LO_SUM
>         addresses won't wrap when offsetting.
>         (rs6000_secondary_reload): Provide secondary reloads needed for
>         wrapping LO_SUM addresses.

Okay.

We just need to tweak it until we close these corner cases.

Do you have a testcase that can be added?

Thanks, David

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 195707)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -5135,17 +5135,14 @@  mem_operand_gpr
   if (TARGET_POWERPC64 && (offset & 3) != 0)
     return false;
 
+  extra = GET_MODE_SIZE (mode) - UNITS_PER_WORD;
+  gcc_assert (extra >= 0);
+
   if (GET_CODE (addr) == LO_SUM)
-    /* We know by alignment that ABI_AIX medium/large model toc refs
-       will not cross a 32k boundary, since all entries in the
-       constant pool are naturally aligned and we check alignment for
-       other medium model toc-relative addresses.  For ABI_V4 and
-       ABI_DARWIN lo_sum addresses, we just check that 64-bit
-       offsets are 4-byte aligned.  */
-    return true;
+    /* For lo_sum addresses, we must allow any offset except one that
+       causes a wrap, so test only the low 16 bits.  */
+    offset = ((offset & 0xffff) ^ 0x8000) - 0x8000;
 
-  extra = GET_MODE_SIZE (mode) - UNITS_PER_WORD;
-  gcc_assert (extra >= 0);
   return offset + 0x8000 < 0x10000u - extra;
 }
 
@@ -13823,19 +13819,31 @@  rs6000_secondary_reload
 	   && MEM_P (x)
 	   && GET_MODE_SIZE (GET_MODE (x)) >= UNITS_PER_WORD)
     {
-      rtx off = address_offset (XEXP (x, 0));
-      unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD;
+      rtx addr = XEXP (x, 0);
+      rtx off = address_offset (addr);
 
-      if (off != NULL_RTX
-	  && (INTVAL (off) & 3) != 0
-	  && (unsigned HOST_WIDE_INT) INTVAL (off) + 0x8000 < 0x10000 - extra)
+      if (off != NULL_RTX)
 	{
-	  if (in_p)
-	    sri->icode = CODE_FOR_reload_di_load;
+	  unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD;
+	  unsigned HOST_WIDE_INT offset = INTVAL (off);
+
+	  /* We need a secondary reload when our legitimate_address_p
+	     says the address is good (as otherwise the entire address
+	     will be reloaded), and the offset is not a multiple of
+	     four.  */
+	  if ((GET_CODE (addr) == LO_SUM
+	       || offset + 0x8000 < 0x10000 - extra)
+	      && (offset & 3) != 0)
+	    {
+	      if (in_p)
+		sri->icode = CODE_FOR_reload_di_load;
+	      else
+		sri->icode = CODE_FOR_reload_di_store;
+	      sri->extra_cost = 2;
+	      ret = NO_REGS;
+	    }
 	  else
-	    sri->icode = CODE_FOR_reload_di_store;
-	  sri->extra_cost = 2;
-	  ret = NO_REGS;
+	    default_p = true;
 	}
       else
 	default_p = true;
@@ -13845,25 +13853,43 @@  rs6000_secondary_reload
 	   && MEM_P (x)
 	   && GET_MODE_SIZE (GET_MODE (x)) > UNITS_PER_WORD)
     {
-      rtx off = address_offset (XEXP (x, 0));
-      unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD;
+      rtx addr = XEXP (x, 0);
+      rtx off = address_offset (addr);
 
-      /* We need a secondary reload only when our legitimate_address_p
-	 says the address is good (as otherwise the entire address
-	 will be reloaded).  So for mode sizes of 8 and 16 this will
-	 be when the offset is in the ranges [0x7ffc,0x7fff] and
-	 [0x7ff4,0x7ff7] respectively.  Note that the address we see
-	 here may have been manipulated by legitimize_reload_address.  */
-      if (off != NULL_RTX
-	  && ((unsigned HOST_WIDE_INT) INTVAL (off) - (0x8000 - extra)
-	      < UNITS_PER_WORD))
+      if (off != NULL_RTX)
 	{
-	  if (in_p)
-	    sri->icode = CODE_FOR_reload_si_load;
+	  unsigned int extra = GET_MODE_SIZE (GET_MODE (x)) - UNITS_PER_WORD;
+	  unsigned HOST_WIDE_INT offset = INTVAL (off);
+
+	  /* We need a secondary reload when our legitimate_address_p
+	     says the address is good (as otherwise the entire address
+	     will be reloaded), and we have a wrap.
+
+	     legitimate_lo_sum_address_p allows LO_SUM addresses to
+	     have any offset so test for wrap in the low 16 bits.
+
+	     legitimate_offset_address_p checks for the range
+	     [-0x8000,0x7fff] for mode size of 8 and [-0x8000,0x7ff7]
+	     for mode size of 16.  We wrap at [0x7ffc,0x7fff] and
+	     [0x7ff4,0x7fff] respectively, so test for the
+	     intersection of these ranges, [0x7ffc,0x7fff] and
+	     [0x7ff4,0x7ff7] respectively.
+
+	     Note that the address we see here may have been
+	     manipulated by legitimize_reload_address.  */
+	  if (GET_CODE (addr) == LO_SUM
+	      ? ((offset & 0xffff) ^ 0x8000) >= 0x10000 - extra
+	      : offset - (0x8000 - extra) < UNITS_PER_WORD)
+	    {
+	      if (in_p)
+		sri->icode = CODE_FOR_reload_si_load;
+	      else
+		sri->icode = CODE_FOR_reload_si_store;
+	      sri->extra_cost = 2;
+	      ret = NO_REGS;
+	    }
 	  else
-	    sri->icode = CODE_FOR_reload_si_store;
-	  sri->extra_cost = 2;
-	  ret = NO_REGS;
+	    default_p = true;
 	}
       else
 	default_p = true;