[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