qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] disas/nanomips: Convert nanoMIPS disassembler to C


From: Peter Maydell
Subject: Re: [PATCH] disas/nanomips: Convert nanoMIPS disassembler to C
Date: Fri, 29 Jul 2022 15:17:55 +0100

On Fri, 29 Jul 2022 at 15:13, Milica Lazarevic
<milica.lazarevic@syrmia.com> wrote:
>
> C++ features like class, exception handling and function overloading
> have been removed and replaced with equivalent C code.
>
> Signed-off-by: Milica Lazarevic <milica.lazarevic@syrmia.com>
> ---
> Please see the discussion about why converting it here:
> https://lists.nongnu.org/archive/html/qemu-devel/2022-06/msg01803.html
>
> The validity of the disassembler after refactoring has been tested with
> the QEMU emulator version 6.0.1. With the most recent version, there is a
> problem with the executing nanoMIPS programs in the semihosting mode. The
> issue is reported here: https://gitlab.com/qemu-project/qemu/-/issues/1126
> We're currently working on fixing this.
>
>  disas/meson.build                  |    2 +-
>  disas/{nanomips.cpp => nanomips.c} | 8407 ++++++++++++++--------------
>  disas/nanomips.h                   | 1076 ----
>  3 files changed, 4154 insertions(+), 5331 deletions(-)
>  rename disas/{nanomips.cpp => nanomips.c} (73%)
>  delete mode 100644 disas/nanomips.h

Is it possible to break this down into smaller pieces so it isn't
one single enormous 5000 line patch ?

I guess partial conversion is likely to run into compilation
difficulties mid-series; if so we could do "disable the
disassembler; convert it; reenable it".

The rationale here is code review -- reviewing a single huge
patch is essentially impossible, so it needs to be broken
down into coherent smaller pieces to be reviewable.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]