[PATCH] sim: h8300: drop separate eightbit memory buffer
Andrew Burgess
andrew.burgess@embecosm.com
Wed Jan 13 20:05:58 GMT 2021
* Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org> [2021-01-12 18:17:04 -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.
That's fine by me.
Thanks,
Andrew
> ---
> sim/h8300/ChangeLog | 14 ++++++-
> sim/h8300/compile.c | 91 +++++++++-----------------------------------
> sim/h8300/sim-main.h | 1 -
> 3 files changed, 32 insertions(+), 74 deletions(-)
>
> diff --git a/sim/h8300/ChangeLog b/sim/h8300/ChangeLog
> index 1f9281fd4b1a..9c3f0710ead5 100644
> --- a/sim/h8300/ChangeLog
> +++ b/sim/h8300/ChangeLog
> @@ -1,3 +1,16 @@
> +2021-01-12 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
> -
> diff --git a/sim/h8300/compile.c b/sim/h8300/compile.c
> index a0e8bbbf8986..47b0577781aa 100644
> --- a/sim/h8300/compile.c
> +++ b/sim/h8300/compile.c
> @@ -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)
> diff --git a/sim/h8300/sim-main.h b/sim/h8300/sim-main.h
> index 3b5ae2adb0a8..b6169b3bc126 100644
> --- a/sim/h8300/sim-main.h
> +++ b/sim/h8300/sim-main.h
> @@ -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;
> --
> 2.28.0
>
More information about the Gdb-patches
mailing list