This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 06 Jan 2016 08:47, Joel Brobecker wrote: > What's still left to do on our end is create a testsuite, which > we haven't gotten around to doing yet. But we will! :-) yeah, i don't think i want to allow any more ports w/out tests. it makes it too hard for me to adjust code w/out breaking things. > +++ b/sim/visium/configure.ac > > +dnl This file is part of GDB. i think all these lines should be changed: dnl This file is part of the GNU Simulators. comes up in a bunch of files in this patch > +SIM_AC_COMMON please add at least: SIM_AC_OPTION_WARNINGS and then fix all the warnings :) > +# BFD conditionally uses zlib, so we must link it in if libbfd does, by > +# using the same condition. > +AM_ZLIB > + > +# BFD uses libdl when when plugins enabled. > +AC_PLUGINS you should not need either of these. they should be handled for you already. > +++ b/sim/visium/sim-fpu.c > > +/* Note: Although the origin of this file has not been researched, > + we know this is not the master copy of this code, and therefore > + we try to do as few modifications as possible, in order to facilitate > + possible coordination with that original, if it is every found. > + This explains why no apparent effort is made to improve this file's > + style to better match our usual standards. */ erm, the origin is pretty clear -- it was duplicated from sim/common/sim-fpu.c. this needs to be rectified. > +++ b/sim/visium/sim-main.h > > + int32_t regs[VISIUM_GENERAL_REGS]; do you want all the regs to be signed ? usually ports have them be unsigned as it makes coding simpler. > +/* A small macro to return the sim_cpu from the sim descriptor. > + We only support one CPU at the moment, so the CPU index is > + always 0. But perhaps we'll need to support SMP on this architecture, > + one day, in which case this macro will be useful to help supporting > + that (easy to find all locations, and perhaps CPU selection could > + be automated inside this macro itself). */ > +#define VISIUM_STATE_CPU(sd) (STATE_CPU (sd, 0)) usually you shouldn't need this. if you have a reference to the state but not a cpu, it tends to indicate the API isn't correctly passing down the cpu as an argument. so those funcs should be adjusted instead. > +++ b/sim/visium/sim_calls.c > > +sim_open (SIM_OPEN_KIND kind, struct host_callback_struct *callback, > + struct bfd *abfd, char **argv) looks like this forgets to call: sim_config sim_post_argv_init > +void > +sim_close (SIM_DESC sd, int quitting) > +{ > + sim_cpu *cpu = VISIUM_STATE_CPU (sd); > + > + visium_core_destroy (sd); > + visium_config_delete (sd); > + > + if (sd->trace_data != NULL) > + visium_trace_close (sd); > +} you should define SIM_CLOSE_HOOK in sim-main.h instead and use the common sim-close.c file. then delete this. > +sim_load (SIM_DESC sd, const char *prog, bfd *abfd, int from_tty) > +sim_read (SIM_DESC sd, SIM_ADDR mem, unsigned char *buf, int length) > +sim_write (SIM_DESC sd, SIM_ADDR mem, const unsigned char *buf, int length) this looks hairy and will require a good bit of unwinding. you shouldn't be defining your own sim_read/sim_write anymore. if you want memory, you should be using the common memory functions to attach it. if you want to simulate hardware, you should be using the WITH_HW framework. > +sim_fetch_register (SIM_DESC sd, int regno, unsigned char *buf, int length) > +sim_store_register (SIM_DESC sd, int regno, unsigned char *buf, int length) don't define these yourself. see commit e1211e55062594679697d2175b7ea77d as an example of converting to the new callbacks. > +sim_stop_reason (SIM_DESC sd, enum sim_stop *reason, int *sigrc) > +sim_stop (SIM_DESC sd) > +sim_resume (SIM_DESC sd, int step, int siggnal) you should be using the common core for these. instead, define sim_engine_run (see bfin/interp.c as a simple example), and instead of setting sd->stop_reason, call sim_engine_halt to halt simulation. that call will not return as it uses setjmp/longjmp. it tends to make design of your sim simpler. > +++ b/sim/visium/visium-defs.h > > +/* Display an error message. */ > +#define err(msg, args ...) \ > + do { \ > + fprintf (stderr, "[ERROR] (%s:%d: errno: %s) " msg "\n", \ > + __FILE__, __LINE__, visium_get_errno_msg (), ## args); \ > + } while (0) you should be using sim_io_eprintf instead of fprintf > +#define info(msg, args ...) \ > + do { \ > + fprintf (stdout, "[INFO] (%s:%d) " msg "\n", __FILE__, __LINE__, \ > + ## args); \ > + } while (0) > + > +/* Display a message. */ > +#define msg(msg, args ...) \ > + do { \ > + fprintf (stdout, msg "\n", ## args); \ > + fflush (stdout); \ > + } while (0) these should use sim_io_printf instead of fprintf > +/* All as "checks" except not requesting the simulation to stop. */ > +#define check(val, msg, args ...) \ > + do { \ > + if (!(val)) \ > + { \ > + err (msg, ## args); \ > + errno = 0; \ > + goto error; \ > + } \ > + } while (0) > + > +/* Check that Addr is not NULL. */ > +#define check_memory(addr) check (addr, "Out of memory") > + > +/* Unconditionally display an error message and jump to an error > + handler. */ > +#define fatal(msg, args ...) \ > + do { \ > + err (msg, ## args); \ > + errno = 0; \ > + goto error; \ > + } while (0) we have sim_io_error for throwing an error and exiting > --- /dev/null > +++ b/sim/visium/visium-dev-syscall.c > > +/* Handle a subset of system calls via a modified newlib C library. > + List of supported functions: > + - open() > + - close() > + - read() > + - write() > + - lseek() > + - rename() > + - unlink() > + - stat() > + - fstat() > + - gettimeofday() > + - isatty() > + - system() > + */ you shouldn't have to implement all this yourself. we have sim-syscall.c to deal with these. all your sim needs to do is handle the trap and then unpack all the regs before passing them to sim_syscall(). > +++ b/sim/visium/visium-ranges.c > +++ b/sim/visium/visium-ranges.h glancing at the API, i think the common sim-arange module provides the same logic already. > +++ b/sim/visium/visium-trace.c > +++ b/sim/visium/visium-trace.h i glanced through the trace logic ... it doesn't seem like it's hardware specific (like you've got a hardware module that is handling this). since it's all software based, you should throw away the visium trace logic and switch to the common sim-trace module. the sim-trace.h header includes a lot of macros to quickly instrument your code. -mike
Attachment:
signature.asc
Description: Digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |