Re: [EXTERNAL] Re: [PATCH v2 2/3] [sim/riscv] Add support for compressed integer instruction set

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

On 19 Dec 2023 06:11, Jaydeep Patil wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> > i haven't been keeping up with riscv specs.  is the compressed extension finalized ?  so you're only implementing official insns in the spec ?  i don't think it's appropriate for the sim to implement vendor-specific stuff at this point in time.
> 
> The compressed instruction set ("c") is not vendor specific. It has been ratified.

that's good.  it's still an extension though and not in the base ISA.  it
shouldn't be available all the time, only when a compatible model is selected
via the --model option.  otherwise it should throw an SIM_SIGILL error.

look at riscv/model_list.def and machs.c files.

> > afaict, there is no relationship between the compression & semi-hosting work.
> > these are just two things you're working on ?  so they don't really need to be in the same patch series.
> 
> Yes, there is no relation between compression & semi-hosting work. The patches are independent and I can re-submit them if needed.

not strictly necessary, but makes it confusing when you label the patch
"semihost", and there's debate on that particular topic.

> > i'm missing something ... why does there need to be tests in gdb at all here ?
> 
> Purpose of riscv-insn-simulation.exp is to test both C extension and semi-hosting. However, I have also added sim specific tests in patch v3.

none of the semihost logic lives in gdb.  it's entirely in the sim code.
there's no need to have any test logic added to gdb -- it can all be in
the sim tree.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]