[PATCH] sim/h8300 different result some addressing

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] sim/h8300 different result some addressing

Yoshinori Sato
Test code
     .h8300h
     mov.l #0x12345678,er0
     mov.w #0xabcd,r1
     mov.w r1,@-er0

Real CPU result
mem 0x345676 -> 0xabcd
reg er0 -> 0x12345676

Sim result
mem 0x345676 -> 0xabcd
reg er0 -> 0x00345676
Sim broke er0 value.

Index: sim/h8300/ChangeLog
===================================================================
RCS file: /cvs/src/src/sim/h8300/ChangeLog,v
retrieving revision 1.63
diff -u -r1.63 ChangeLog
--- sim/h8300/ChangeLog 22 Aug 2009 16:56:53 -0000 1.63
+++ sim/h8300/ChangeLog 23 Nov 2009 19:17:06 -0000
@@ -1,3 +1,9 @@
+2009-11-23  Yoshinori Sato <[hidden email]>
+ * compile.c(fetch_1): Fix pre-dec, pre-inc, post-dec and post-inc.
+ Index registers not masked memory areas.
+ Only simply increment or decrement.
+ * compile.c(store_1): Ditto.
+
 2009-08-22  Ralf Wildenhues  <[hidden email]>
 
  * config.in: Regenerate.
Index: sim/h8300/compile.c
===================================================================
RCS file: /cvs/src/src/sim/h8300/compile.c,v
retrieving revision 1.45
diff -u -r1.45 compile.c
--- sim/h8300/compile.c 1 Dec 2008 16:10:45 -0000 1.45
+++ sim/h8300/compile.c 23 Nov 2009 19:17:08 -0000
@@ -1386,105 +1386,93 @@
       break;
     case X (OP_POSTINC, SB): /* Register indirect w/post-incr: byte.  */
       t = GET_L_REG (rn);
-      t &= h8_get_mask (sd);
-      r = GET_MEMORY_B (t);
+      r = GET_MEMORY_B (t & h8_get_mask (sd));
       if (!twice)
  t += 1;
-      t = t & h8_get_mask (sd);
       SET_L_REG (rn, t);
       *val = r;
       break;
     case X (OP_POSTINC, SW): /* Register indirect w/post-incr: word.  */
       t = GET_L_REG (rn);
-      t &= h8_get_mask (sd);
-      r = GET_MEMORY_W (t);
+      r = GET_MEMORY_W (t & h8_get_mask (sd));
       if (!twice)
  t += 2;
-      t = t & h8_get_mask (sd);
       SET_L_REG (rn, t);
       *val = r;
       break;
     case X (OP_POSTINC, SL): /* Register indirect w/post-incr: long.  */
       t = GET_L_REG (rn);
-      t &= h8_get_mask (sd);
-      r = GET_MEMORY_L (t);
+      r = GET_MEMORY_L (t & h8_get_mask (sd));
       if (!twice)
  t += 4;
-      t = t & h8_get_mask (sd);
       SET_L_REG (rn, t);
       *val = r;
       break;
 
     case X (OP_POSTDEC, SB): /* Register indirect w/post-decr: byte.  */
       t = GET_L_REG (rn);
-      t &= h8_get_mask (sd);
-      r = GET_MEMORY_B (t);
+      r = GET_MEMORY_B (t & h8_get_mask (sd));
       if (!twice)
  t -= 1;
-      t = t & h8_get_mask (sd);
       SET_L_REG (rn, t);
       *val = r;
       break;
     case X (OP_POSTDEC, SW): /* Register indirect w/post-decr: word.  */
       t = GET_L_REG (rn);
-      t &= h8_get_mask (sd);
-      r = GET_MEMORY_W (t);
+      r = GET_MEMORY_W (t & h8_get_mask (sd));
       if (!twice)
  t -= 2;
-      t = t & h8_get_mask (sd);
       SET_L_REG (rn, t);
       *val = r;
       break;
     case X (OP_POSTDEC, SL): /* Register indirect w/post-decr: long.  */
       t = GET_L_REG (rn);
-      t &= h8_get_mask (sd);
-      r = GET_MEMORY_L (t);
+      r = GET_MEMORY_L (t & h8_get_mask (sd));
       if (!twice)
  t -= 4;
-      t = t & h8_get_mask (sd);
       SET_L_REG (rn, t);
       *val = r;
       break;
 
     case X (OP_PREDEC, SB): /* Register indirect w/pre-decr: byte.  */
       t = GET_L_REG (rn) - 1;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       *val = GET_MEMORY_B (t);
       break;
       
     case X (OP_PREDEC, SW): /* Register indirect w/pre-decr: word.  */
       t = GET_L_REG (rn) - 2;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       *val = GET_MEMORY_W (t);
       break;
       
     case X (OP_PREDEC, SL): /* Register indirect w/pre-decr: long.  */
       t = GET_L_REG (rn) - 4;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       *val = GET_MEMORY_L (t);
       break;
       
     case X (OP_PREINC, SB): /* Register indirect w/pre-incr: byte.  */
       t = GET_L_REG (rn) + 1;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       *val = GET_MEMORY_B (t);
       break;
 
     case X (OP_PREINC, SW): /* Register indirect w/pre-incr: long.  */
       t = GET_L_REG (rn) + 2;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       *val = GET_MEMORY_W (t);
       break;
 
     case X (OP_PREINC, SL): /* Register indirect w/pre-incr: long.  */
       t = GET_L_REG (rn) + 4;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       *val = GET_MEMORY_L (t);
       break;
 
@@ -1621,8 +1609,8 @@
       t = GET_L_REG (rn);
       if (!twice)
  t -= 1;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       SET_MEMORY_B (t, n);
 
       break;
@@ -1630,8 +1618,8 @@
       t = GET_L_REG (rn);
       if (!twice)
  t -= 2;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       SET_MEMORY_W (t, n);
       break;
 
@@ -1639,8 +1627,8 @@
       t = GET_L_REG (rn);
       if (!twice)
  t -= 4;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       SET_MEMORY_L (t, n);
       break;
 
@@ -1648,8 +1636,8 @@
       t = GET_L_REG (rn);
       if (!twice)
  t += 1;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       SET_MEMORY_B (t, n);
 
       break;
@@ -1657,8 +1645,8 @@
       t = GET_L_REG (rn);
       if (!twice)
  t += 2;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       SET_MEMORY_W (t, n);
       break;
 
@@ -1666,45 +1654,51 @@
       t = GET_L_REG (rn);
       if (!twice)
  t += 4;
-      t &= h8_get_mask (sd);
       SET_L_REG (rn, t);
+      t &= h8_get_mask (sd);
       SET_MEMORY_L (t, n);
       break;
 
     case X (OP_POSTDEC, SB): /* Register indirect w/post-decr, byte.  */
-      t = GET_L_REG (rn) & h8_get_mask (sd);
-      SET_MEMORY_B (t, n);
+      t = GET_L_REG (rn);
       SET_L_REG (rn, t - 1);
+      t &= h8_get_mask (sd);
+      SET_MEMORY_B (t, n);
       break;
 
     case X (OP_POSTDEC, SW): /* Register indirect w/post-decr, word.  */
-      t = GET_L_REG (rn) & h8_get_mask (sd);
-      SET_MEMORY_W (t, n);
+      t = GET_L_REG (rn);
       SET_L_REG (rn, t - 2);
+      t &= h8_get_mask (sd);
+      SET_MEMORY_W (t, n);
       break;
 
     case X (OP_POSTDEC, SL): /* Register indirect w/post-decr, long.  */
-      t = GET_L_REG (rn) & h8_get_mask (sd);
-      SET_MEMORY_L (t, n);
+      t = GET_L_REG (rn);
       SET_L_REG (rn, t - 4);
+      t &= h8_get_mask (sd);
+      SET_MEMORY_L (t, n);
       break;
 
     case X (OP_POSTINC, SB): /* Register indirect w/post-incr, byte.  */
-      t = GET_L_REG (rn) & h8_get_mask (sd);
-      SET_MEMORY_B (t, n);
+      t = GET_L_REG (rn);
       SET_L_REG (rn, t + 1);
+      t &= h8_get_mask (sd);
+      SET_MEMORY_B (t, n);
       break;
 
     case X (OP_POSTINC, SW): /* Register indirect w/post-incr, word.  */
-      t = GET_L_REG (rn) & h8_get_mask (sd);
-      SET_MEMORY_W (t, n);
+      t = GET_L_REG (rn);
       SET_L_REG (rn, t + 2);
+      t &= h8_get_mask (sd);
+      SET_MEMORY_W (t, n);
       break;
 
     case X (OP_POSTINC, SL): /* Register indirect w/post-incr, long.  */
-      t = GET_L_REG (rn) & h8_get_mask (sd);
-      SET_MEMORY_L (t, n);
+      t = GET_L_REG (rn);
       SET_L_REG (rn, t + 4);
+      t &= h8_get_mask (sd);
+      SET_MEMORY_L (t, n);
       break;
 
     case X (OP_DISP, SB): /* Register indirect w/displacement, byte.  */

--
Yoshinori Sato
<[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] sim/h8300 different result some addressing

Joel Brobecker
> +2009-11-23  Yoshinori Sato <[hidden email]>
> + * compile.c(fetch_1): Fix pre-dec, pre-inc, post-dec and post-inc.
> + Index registers not masked memory areas.
> + Only simply increment or decrement.
> + * compile.c(store_1): Ditto.

I don't really know this CPU, and you're the only one to have made
a "recent" (2007) change to this CPU (excluding basic configure
regeneration).  And since there is no apparent maintainer for that
target, we're going to have to trust you on this.

OK.
--
Joel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] sim/h8300 different result some addressing

Yoshinori Sato
At Tue, 24 Nov 2009 14:16:00 -0800,
Joel Brobecker wrote:

>
> > +2009-11-23  Yoshinori Sato <[hidden email]>
> > + * compile.c(fetch_1): Fix pre-dec, pre-inc, post-dec and post-inc.
> > + Index registers not masked memory areas.
> > + Only simply increment or decrement.
> > + * compile.c(store_1): Ditto.
>
> I don't really know this CPU, and you're the only one to have made
> a "recent" (2007) change to this CPU (excluding basic configure
> regeneration).  And since there is no apparent maintainer for that
> target, we're going to have to trust you on this.
>
> OK.
> --
> Joel

This CPU instruction manual is here.
http://documentation.renesas.com/eng/products/mpumcu/rej09b0139_h8s2600.pdf

Please conferm this fix.

--
Yoshinori Sato
<[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] sim/h8300 different result some addressing

Joel Brobecker
> > > +2009-11-23  Yoshinori Sato <[hidden email]>
> > > + * compile.c(fetch_1): Fix pre-dec, pre-inc, post-dec and post-inc.
> > > + Index registers not masked memory areas.
> > > + Only simply increment or decrement.
> > > + * compile.c(store_1): Ditto.

OK.

> This CPU instruction manual is here.
> http://documentation.renesas.com/eng/products/mpumcu/rej09b0139_h8s2600.pdf

I do appreciate being pointed to the documentation, and I learnt
a new syntax I hadn't seen before. But when I say I will trust you
on something, I really mean it. So feel free to commit when I approve
a patch based on this.

--
Joel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] sim/h8300 different result some addressing

Yoshinori Sato
At Tue, 1 Dec 2009 10:58:09 -0800,
Joel Brobecker wrote:

>
> > > > +2009-11-23  Yoshinori Sato <[hidden email]>
> > > > + * compile.c(fetch_1): Fix pre-dec, pre-inc, post-dec and post-inc.
> > > > + Index registers not masked memory areas.
> > > > + Only simply increment or decrement.
> > > > + * compile.c(store_1): Ditto.
>
> OK.
>
> > This CPU instruction manual is here.
> > http://documentation.renesas.com/eng/products/mpumcu/rej09b0139_h8s2600.pdf
>
> I do appreciate being pointed to the documentation, and I learnt
> a new syntax I hadn't seen before. But when I say I will trust you
> on something, I really mean it. So feel free to commit when I approve
> a patch based on this.
>

OK.
Can I commit this?

> --
> Joel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] sim/h8300 different result some addressing

Joel Brobecker
> OK.
> Can I commit this?

Yes (this is what the somewhat short-and-cryptic "OK" meant).
It's nice to see that you're being careful.

--
Joel