Patchwork [04/28] mn10300: Emit the movm stores in the correct order.

login
register
mail settings
Submitter Richard Henderson
Date Jan. 10, 2011, 8:31 p.m.
Message ID <1294691517-19580-5-git-send-email-rth@redhat.com>
Download mbox | patch
Permalink /patch/78202/
State New
Headers show

Comments

Richard Henderson - Jan. 10, 2011, 8:31 p.m.
From: Richard Henderson <rth@twiddle.net>

---
 gcc/config/mn10300/mn10300-protos.h |    1 -
 gcc/config/mn10300/mn10300.c        |  110 ++++++++++++++++++-----------------
 2 files changed, 56 insertions(+), 55 deletions(-)
Jeff Law - Jan. 11, 2011, 2:51 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/10/11 13:31, Richard Henderson wrote:
I'll assume that you verified the movm fix somehow?  I recall the
original mn103 docs being somewhat ambiguous in how movm was documented
leading to much confusion between the Matsushita engineers & myself
(particularly since GCC & the simulator were self-consistent, but
incorrect relative to the hardware).

I thought we had all that sorted out back in 1999 or so :-)


jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNLG5sAAoJEBRtltQi2kC788oH/RNhV2ANSm0gp3gWBJB3kRXI
NhKIpUizckowp5aI38yPK5xWbGrVkuWTGVSuzwDqZa8TLOfaj8jejo98v2VA5DN9
yPOu7FlNhA05gAv5PLJWka99YkDsfUZYDmcUkCdLBSJQEkWTjV0PCCQ/0BYKEw3d
yG1gexGpX/lVYZSRHAqHBFLqN+dwHZ5LpILLwP6ws9CeCzoMGfbVIeYoyHwjehME
DVxrpGszalz0G1kjeo2CZzdCCkaCNnxNJubjO9NVFtGORTvkCegTLnPvKJOz4R13
dq/hovoKPWc/VC1bXDjLy1iv55MtId8DICxNUu9gvfhOXAAt4mc9oigBwR7/z24=
=dtom
-----END PGP SIGNATURE-----
Richard Henderson - Jan. 11, 2011, 4:45 p.m.
On 01/11/2011 06:51 AM, Jeff Law wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 01/10/11 13:31, Richard Henderson wrote:
> I'll assume that you verified the movm fix somehow?  I recall the
> original mn103 docs being somewhat ambiguous in how movm was documented
> leading to much confusion between the Matsushita engineers & myself
> (particularly since GCC & the simulator were self-consistent, but
> incorrect relative to the hardware).

Well, the change matches both the documentation and the simulator.
I can only assume both match the hardware atm...  ;-)


r~

Patch

diff --git a/gcc/config/mn10300/mn10300-protos.h b/gcc/config/mn10300/mn10300-protos.h
index ebf915e..d787dcb 100644
--- a/gcc/config/mn10300/mn10300-protos.h
+++ b/gcc/config/mn10300/mn10300-protos.h
@@ -27,7 +27,6 @@ 
 extern rtx   mn10300_legitimize_pic_address (rtx, rtx);
 extern int   mn10300_legitimate_pic_operand_p (rtx);
 extern bool  mn10300_function_value_regno_p (const unsigned int);
-extern void  mn10300_gen_multiple_store (int);
 extern int   mn10300_get_live_callee_saved_regs (void);
 extern bool  mn10300_hard_regno_mode_ok (unsigned int, Mmode);
 extern bool  mn10300_legitimate_constant_p (rtx);
diff --git a/gcc/config/mn10300/mn10300.c b/gcc/config/mn10300/mn10300.c
index 23b198a..534ddaf 100644
--- a/gcc/config/mn10300/mn10300.c
+++ b/gcc/config/mn10300/mn10300.c
@@ -689,51 +689,61 @@  F (rtx r)
 	                       (const_int -N*4)))
 	      (reg:SI R1))) */
 
-void
-mn10300_gen_multiple_store (int mask)
+static void
+mn10300_gen_multiple_store (unsigned int mask)
 {
-  if (mask != 0)
+  /* The order in which registers are stored, from SP-4 through SP-N*4.  */
+  static const unsigned int store_order[8] = {
+    /* e2, e3: never saved */
+    FIRST_EXTENDED_REGNUM + 4,
+    FIRST_EXTENDED_REGNUM + 5,
+    FIRST_EXTENDED_REGNUM + 6,
+    FIRST_EXTENDED_REGNUM + 7,
+    /* e0, e1, mdrq, mcrh, mcrl, mcvf: never saved. */
+    FIRST_DATA_REGNUM + 2,
+    FIRST_DATA_REGNUM + 3,
+    FIRST_ADDRESS_REGNUM + 2,
+    FIRST_ADDRESS_REGNUM + 3,
+    /* d0, d1, a0, a1, mdr, lir, lar: never saved.  */
+  };
+
+  rtx x, elts[9];
+  unsigned int i;
+  int count;
+
+  if (mask == 0)
+    return;
+
+  for (i = count = 0; i < ARRAY_SIZE(store_order); ++i)
     {
-      int i;
-      int count;
-      rtx par;
-      int pari;
-
-      /* Count how many registers need to be saved.  */
-      count = 0;
-      for (i = 0; i <= LAST_EXTENDED_REGNUM; i++)
-	if ((mask & (1 << i)) != 0)
-	  count += 1;
-
-      /* We need one PARALLEL element to update the stack pointer and
-	 an additional element for each register that is stored.  */
-      par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (count + 1));
-
-      /* Create the instruction that updates the stack pointer.  */
-      XVECEXP (par, 0, 0)
-	= F (gen_rtx_SET (SImode,
-			  stack_pointer_rtx,
-			  gen_rtx_PLUS (SImode,
-					stack_pointer_rtx,
-					GEN_INT (-count * 4))));
-
-      /* Create each store.  */
-      pari = 1;
-      for (i = LAST_EXTENDED_REGNUM; i >= 0; i--)
-	if ((mask & (1 << i)) != 0)
-	  {
-	    rtx address = gen_rtx_PLUS (SImode,
-					stack_pointer_rtx,
-					GEN_INT (-pari * 4));
-	    XVECEXP(par, 0, pari)
-	      = F (gen_rtx_SET (VOIDmode,
-				gen_rtx_MEM (SImode, address),
-				gen_rtx_REG (SImode, i)));
-	    pari += 1;
-	  }
+      unsigned regno = store_order[i];
+
+      if (((mask >> regno) & 1) == 0)
+	continue;
+
+      ++count;
+      x = plus_constant (stack_pointer_rtx, count * -4);
+      x = gen_frame_mem (SImode, x);
+      x = gen_rtx_SET (VOIDmode, x, gen_rtx_REG (SImode, regno));
+      elts[count] = F(x);
 
-      F (emit_insn (par));
+      /* Remove the register from the mask so that... */
+      mask &= ~(1u << regno);
     }
+
+  /* ... we can make sure that we didn't try to use a register
+     not listed in the store order.  */
+  gcc_assert (mask == 0);
+
+  /* Create the instruction that updates the stack pointer.  */
+  x = plus_constant (stack_pointer_rtx, count * -4);
+  x = gen_rtx_SET (VOIDmode, stack_pointer_rtx, x);
+  elts[0] = F(x);
+
+  /* We need one PARALLEL element to update the stack pointer and
+     an additional element for each register that is stored.  */
+  x = gen_rtx_PARALLEL (VOIDmode, gen_rtvec_v (count + 1, elts));
+  F (emit_insn (x));
 }
 
 void
@@ -1273,27 +1283,19 @@  mn10300_store_multiple_operation (rtx op,
       || INTVAL (XEXP (elt, 1)) != -(count - 1) * 4)
     return 0;
 
-  /* Now go through the rest of the vector elements.  They must be
-     ordered so that the first instruction stores the highest-numbered
-     register to the highest stack slot and that subsequent instructions
-     store a lower-numbered register to the slot below.
-
-     LAST keeps track of the smallest-numbered register stored so far.
-     MASK is the set of stored registers.  */
-  last = LAST_EXTENDED_REGNUM + 1;
   mask = 0;
   for (i = 1; i < count; i++)
     {
-      /* Check that element i is a (set (mem M) R) and that R is valid.  */
+      /* Check that element i is a (set (mem M) R).  */
+      /* ??? Validate the register order a-la mn10300_gen_multiple_store.
+	 Remember: the ordering is *not* monotonic.  */
       elt = XVECEXP (op, 0, i);
       if (GET_CODE (elt) != SET
 	  || (! MEM_P (SET_DEST (elt)))
-	  || (! REG_P (SET_SRC (elt)))
-	  || REGNO (SET_SRC (elt)) >= last)
+	  || (! REG_P (SET_SRC (elt))))
 	return 0;
 
-      /* R was OK, so provisionally add it to MASK.  We return 0 in any
-	 case if the rest of the instruction has a flaw.  */
+      /* Remember which registers are to be saved.  */
       last = REGNO (SET_SRC (elt));
       mask |= (1 << last);