Re: [RFC][PATCH 2/2] RISC-V: Add cgen-based simulator port

[Jim Wilson <jimw at sifive dot com>]

On Thu, Sep 12, 2019 at 4:39 AM Ed Jones <ed.jones@embecosm.com> wrote:
> This adds a riscv port of the simulator, based on the cgen based cpu
> description.

I'm not a gdb maintainer, so I can't formally approve the gdb bits.

We will need to specify maintainers for this code.  I'm willing to be a
secondary maintainer, but presumably someone at Embecosm will want to be
the primary maintainer, possibly Edward Jones since he submitted the patches.

include/gdb/sim-riscv.h copies a table from gdb/riscv-tdep.h, having two
copies of this table isn't wise, there should be a single copy that both
gdb and sim can use, it mentions RiscV, RISC-V is the official spelling

Looks like the simulator only builds one of 32-bit or 64-bit at a time.  The
rest of the toolchain supports both at the same time.  This will be
inconvenient for some use cases.

sim/riscv/riscv.c has a function for each supported architecture variant
which looks inconvenient, as it means we have to specify every single one.

The simulator testcases are really nice.  That is actually a lot more work
than I would have expected for a simulator testsuite.

The simulator testsuite only works if I configure a rv64gc toolchain.  If
I configure a rv32i toolchain for instance, then all of the *w, compressed,
FP, atomic, and mul/div tests fail.  This should be fixable by specifying
an assembler -march=X option for the minimum arch required by a test, as the
toolchain supports all arch choices no matter what it is configured for.

When I try to use the simulator inside gdb, for a riscv32-elf target, without
specifying an executable first, I get
gamma05:2057$ build-install/bin/riscv32-unknown-elf-gdb -q
(gdb) target sim
sim-model.c:210: assertion failed - model != NULL
(gdb)
It does work if I start gdb with an executable though.

Otherwise this looks OK to me.

The other simulator written by Mike Frysinger doesn't have the
architecture selection issues as the tests are all hand written, but
being hand written modifying it may be more involved than modifying
the cgen simulator.

Jim