sim: h8300: drop separate eightbit memory buffer
authorMike Frysinger <vapier@gentoo.org>
Thu, 18 Aug 2016 06:05:03 +0000 (23:05 -0700)
committerMike Frysinger <vapier@gentoo.org>
Thu, 14 Jan 2021 02:54:00 +0000 (21:54 -0500)
The h8300 sim has its own implementation for memory handling that I'd
like to replace with the common sim memory code.  However, it's got a
weird bit of code it calls "eightbit mem" that makes this not as easy
as it would otherwise be.  The code has this comment:
/* These define the size of main memory for the simulator.

   Note the size of main memory for the H8/300H is only 256k.  Keeping it
   small makes the simulator run much faster and consume less memory.

   The linker knows about the limited size of the simulator's main memory
   on the H8/300H (via the h8300h.sc linker script).  So if you change
   H8300H_MSIZE, be sure to fix the linker script too.

   Also note that there's a separate "eightbit" area aside from main
   memory.  For simplicity, the simulator assumes any data memory reference
   outside of main memory refers to the eightbit area (in theory, this
   can only happen when simulating H8/300H programs).  We make no attempt
   to catch overlapping addresses, wrapped addresses, etc etc.  */

I've read the H8/300 Programming Manual and the H8/300H Software Manual
and can't find documentation on it.  The closest I can find is the bits
about the exception vectors, but that sounds like a convention where the
first 256 bytes of memory are used for a special purpose.  The sim will
actually allocate a sep memory buffer of 256 bytes and you address it by
accessing anywhere outside of main memory.  e.g. I would expect code to
access it like:
uint32_t *data = (void *)0;
data[0] = reset_exception_vector;
not like the sim expects like:
uint8_t *data = (void *)0x1000000;
data[0] = ...;

The gcc manual has an "eightbit_data" attribute:
Use this attribute on the H8/300, H8/300H, and H8S to indicate that
the specified variable should be placed into the eight-bit data
section. The compiler generates more efficient code for certain
operations on data in the eight-bit data area. Note the eight-bit
data area is limited to 256 bytes of data.

And the gcc code implies that it's accessed via special addressing:
   eightbit_data: This variable lives in the 8-bit data area and can
   be referenced with 8-bit absolute memory addresses.

I'm fairly certain these are referring to the 8-bit addressing modes
that allow access to 0xff00 - 0xffff with only an 8-bit immediate.
They aren't completely separate address spaces which this eightbit
memory buffer occupies.

But the sim doesn't access its eightbit memory based on specific insns,
it does it purely on the addresses requested.

Unfortunately, much of this code was authored by Michael Snyder, so I
can't ask him :(.  I asked Renesas support and they didn't know:
https://renesasrulz.com/the_vault/f/archive-forum/6952/question-about-eightbit-memory

So I've come to the conclusion that this was a little sim-specific hack
done for <some convenience> and has no relation to real hardware.  And
as such, let's drop it until someone notices and can provide a reason
for why we need to support it.

sim/h8300/ChangeLog
sim/h8300/compile.c
sim/h8300/sim-main.h

index 1f9281fd4b1ab7147dcc9b84a8567504e21d87a6..f4d7710f7cb4fab2b07914ff497d02891bd9e697 100644 (file)
@@ -1,3 +1,16 @@
+2021-01-13  Mike Frysinger  <vapier@gentoo.org>
+
+       * compile.c (memory_size): Move definition to top of file.
+       (h8_get_memory, h8_set_memory): Assert access is within memory_size.
+       (h8_get_eightbit_buf): Delete.
+       h8_set_eightbit_buf, h8_get_eightbit, h8_set_eightbit): Likewise.
+       (GET_MEMORY_L): Delete eightbit references.
+       (GET_MEMORY_W, GET_MEMORY_B, SET_MEMORY_L, SET_MEMORY_W,
+       SET_MEMORY_B, init_pointers, step_once, sim_load): Likewise.
+       (sim_write): Likewise.  Return i instead of size.
+       (sim_read): Check addr is within memory_size.
+       * sim-main.h (struct h8300_cpu_state): Delete eightbit.
+
 2021-01-11  Mike Frysinger  <vapier@gentoo.org>
 
        * configure.ac: Call SIM_AC_OPTION_WARNINGS.
@@ -1287,4 +1300,3 @@ Sun Jan  3 14:15:07 1993  Steve Chamberlain  (sac@thepub.cygnus.com)
 Tue Dec 22 13:56:48 1992  Steve Chamberlain  (sac@thepub.cygnus.com)
 
        * new
-
index a0e8bbbf89865b640beea4bd9c75c4012b80273e..47b0577781aaa65bad717c51e163d48e7dd6189d 100644 (file)
@@ -38,6 +38,8 @@
 
 int debug;
 
+static int memory_size;
+
 #define X(op, size)  (op * 4 + size)
 
 #define SP (h8300hmode && !h8300_normal_mode ? SL : SW)
@@ -251,39 +253,17 @@ h8_set_memory_buf (SIM_DESC sd, unsigned char *ptr)
 static unsigned char
 h8_get_memory (SIM_DESC sd, int idx)
 {
+  ASSERT (idx < memory_size);
   return (STATE_CPU (sd, 0)) -> memory[idx];
 }
 
 static void
 h8_set_memory (SIM_DESC sd, int idx, unsigned int val)
 {
+  ASSERT (idx < memory_size);
   (STATE_CPU (sd, 0)) -> memory[idx] = (unsigned char) val;
 }
 
-static unsigned char *
-h8_get_eightbit_buf (SIM_DESC sd)
-{
-  return (STATE_CPU (sd, 0)) -> eightbit;
-}
-
-static void
-h8_set_eightbit_buf (SIM_DESC sd, unsigned char *ptr)
-{
-  (STATE_CPU (sd, 0)) -> eightbit = ptr;
-}
-
-static unsigned char
-h8_get_eightbit (SIM_DESC sd, int idx)
-{
-  return (STATE_CPU (sd, 0)) -> eightbit[idx];
-}
-
-static void
-h8_set_eightbit (SIM_DESC sd, int idx, unsigned int val)
-{
-  (STATE_CPU (sd, 0)) -> eightbit[idx] = (unsigned char) val;
-}
-
 static unsigned int
 h8_get_delayed_branch (SIM_DESC sd)
 {
@@ -424,8 +404,6 @@ int h8300smode  = 0;
 int h8300_normal_mode  = 0;
 int h8300sxmode = 0;
 
-static int memory_size;
-
 static int
 get_now (void)
 {
@@ -1151,41 +1129,31 @@ static unsigned short *wreg[16];
   ((X) < memory_size \
    ? ((h8_get_memory (sd, (X)+0) << 24) | (h8_get_memory (sd, (X)+1) << 16)  \
     | (h8_get_memory (sd, (X)+2) <<  8) | (h8_get_memory (sd, (X)+3) <<  0)) \
-   : ((h8_get_eightbit (sd, ((X)+0) & 0xff) << 24) \
-    | (h8_get_eightbit (sd, ((X)+1) & 0xff) << 16) \
-    | (h8_get_eightbit (sd, ((X)+2) & 0xff) <<  8) \
-    | (h8_get_eightbit (sd, ((X)+3) & 0xff) <<  0)))
+   : 0)
 
 #define GET_MEMORY_W(X) \
   ((X) < memory_size \
-   ? ((h8_get_memory   (sd, (X)+0) << 8) \
-    | (h8_get_memory   (sd, (X)+1) << 0)) \
-   : ((h8_get_eightbit (sd, ((X)+0) & 0xff) << 8) \
-    | (h8_get_eightbit (sd, ((X)+1) & 0xff) << 0)))
-
+   ? ((h8_get_memory (sd, (X)+0) << 8) | (h8_get_memory (sd, (X)+1) << 0)) \
+   : 0)
 
 #define GET_MEMORY_B(X) \
-  ((X) < memory_size ? (h8_get_memory   (sd, (X))) \
-                     : (h8_get_eightbit (sd, (X) & 0xff)))
+  ((X) < memory_size ? h8_get_memory (sd, (X)) : 0)
 
 #define SET_MEMORY_L(X, Y)  \
 {  register unsigned char *_p; register int __y = (Y); \
-   _p = ((X) < memory_size ? h8_get_memory_buf   (sd) +  (X) : \
-                             h8_get_eightbit_buf (sd) + ((X) & 0xff)); \
+   _p = ((X) < memory_size ? h8_get_memory_buf (sd) + (X) : 0); \
    _p[0] = __y >> 24; _p[1] = __y >> 16; \
    _p[2] = __y >>  8; _p[3] = __y >>  0; \
 }
 
 #define SET_MEMORY_W(X, Y) \
 {  register unsigned char *_p; register int __y = (Y); \
-   _p = ((X) < memory_size ? h8_get_memory_buf   (sd) +  (X) : \
-                             h8_get_eightbit_buf (sd) + ((X) & 0xff)); \
+   _p = ((X) < memory_size ? h8_get_memory_buf (sd) + (X) : 0); \
    _p[0] = __y >> 8; _p[1] = __y; \
 }
 
 #define SET_MEMORY_B(X, Y) \
-  ((X) < memory_size ? (h8_set_memory   (sd, (X), (Y))) \
-                     : (h8_set_eightbit (sd, (X) & 0xff, (Y))))
+  ((X) < memory_size ? h8_set_memory (sd, (X), (Y)) : 0)
 
 /* Simulate a memory fetch.
    Return 0 for success, -1 for failure.
@@ -1661,13 +1629,10 @@ init_pointers (SIM_DESC sd)
 
       if (h8_get_memory_buf (sd))
        free (h8_get_memory_buf (sd));
-      if (h8_get_eightbit_buf (sd))
-       free (h8_get_eightbit_buf (sd));
 
       h8_set_memory_buf (sd, (unsigned char *) 
                         calloc (sizeof (char), memory_size));
       sd->memory_size = memory_size;
-      h8_set_eightbit_buf (sd, (unsigned char *) calloc (sizeof (char), 256));
 
       h8_set_mask (sd, memory_size - 1);
 
@@ -2164,25 +2129,12 @@ step_once (SIM_DESC sd, SIM_CPU *cpu)
                                    ? h8_get_reg (sd, R4_REGNUM) & 0xffff
                                    : h8_get_reg (sd, R4_REGNUM) & 0xff);
 
-             _src = (h8_get_reg (sd, R5_REGNUM) < memory_size
-                     ? h8_get_memory_buf   (sd) + h8_get_reg (sd, R5_REGNUM)
-                     : h8_get_eightbit_buf (sd) + 
-                      (h8_get_reg (sd, R5_REGNUM) & 0xff));
+             _src = h8_get_memory_buf (sd) + h8_get_reg (sd, R5_REGNUM);
              if ((_src + count) >= (h8_get_memory_buf (sd) + memory_size))
-               {
-                 if ((_src + count) >= (h8_get_eightbit_buf (sd) + 0x100))
-                   goto illegal;
-               }
-             _dst = (h8_get_reg (sd, R6_REGNUM) < memory_size
-                     ? h8_get_memory_buf   (sd) + h8_get_reg (sd, R6_REGNUM)
-                     : h8_get_eightbit_buf (sd) + 
-                      (h8_get_reg (sd, R6_REGNUM) & 0xff));
-
+               goto illegal;
+             _dst = h8_get_memory_buf (sd) + h8_get_reg (sd, R6_REGNUM);
              if ((_dst + count) >= (h8_get_memory_buf (sd) + memory_size))
-               {
-                 if ((_dst + count) >= (h8_get_eightbit_buf (sd) + 0x100))
-                   goto illegal;
-               }
+               goto illegal;
              memcpy (_dst, _src, count);
 
              h8_set_reg (sd, R5_REGNUM, h8_get_reg (sd, R5_REGNUM) + count);
@@ -4444,11 +4396,9 @@ sim_write (SIM_DESC sd, SIM_ADDR addr, const unsigned char *buffer, int size)
          h8_set_memory    (sd, addr + i, buffer[i]);
        }
       else
-       {
-         h8_set_eightbit (sd, (addr + i) & 0xff, buffer[i]);
-       }
+       break;
     }
-  return size;
+  return i;
 }
 
 int
@@ -4457,10 +4407,10 @@ sim_read (SIM_DESC sd, SIM_ADDR addr, unsigned char *buffer, int size)
   init_pointers (sd);
   if (addr < 0)
     return 0;
-  if (addr < memory_size)
+  if (addr + size < memory_size)
     memcpy (buffer, h8_get_memory_buf (sd) + addr, size);
   else
-    memcpy (buffer, h8_get_eightbit_buf (sd) + (addr & 0xff), size);
+    return 0;
   return size;
 }
 
@@ -4835,13 +4785,10 @@ sim_load (SIM_DESC sd, const char *prog, bfd *abfd, int from_tty)
 
   if (h8_get_memory_buf (sd))
     free (h8_get_memory_buf (sd));
-  if (h8_get_eightbit_buf (sd))
-    free (h8_get_eightbit_buf (sd));
 
   h8_set_memory_buf (sd, (unsigned char *) 
                     calloc (sizeof (char), memory_size));
   sd->memory_size = memory_size;
-  h8_set_eightbit_buf (sd, (unsigned char *) calloc (sizeof (char), 256));
 
   /* `msize' must be a power of two.  */
   if ((memory_size & (memory_size - 1)) != 0)
index 3b5ae2adb0a86d44486d55d3f6d5ce24c457d48e..b6169b3bc126715efa85387b32047a95a2fc0040 100644 (file)
@@ -126,7 +126,6 @@ struct _sim_cpu {
   char **command_line;         /* Pointer to command line arguments.  */
 
   unsigned char *memory;
-  unsigned char *eightbit;
   int mask;
   
   sim_cpu_base base;