Re: [Qemu-devel] [PATCH v2 0/5] disas: add libvixl to support A64 disassembly

[Laurent Desnogues]

On Tue, Jan 28, 2014 at 12:45 PM, Peter Maydell
<address@hidden> wrote:
> Ping for review/testing/comments on this version, please?

I still dislike the idea of importing so much code in particular for
something that is incomplete:  as far as I can see, AdvSIMD
instructions are not supported.  The very least that should be
done would be to add a file that gives a rough status of what is
and what is not implemented.

And what if the vixl authors never implement AdvSIMD?  This
is the most difficult part of Aarch64 to disassemble (integer and
FP instructions are really easy to disassemble).  Will someone
add these and will vixl authors accept the changes or will we
then start diverging from vixl implementation?  Is vixl even still
supported or in development (no commit for 6 months)?

We need an Aarch64 disassembler, but IMHO these questions
should be answered before proceeding.


Laurent

> thanks
> -- PMM
>
> On 16 January 2014 11:13, Peter Maydell <address@hidden> wrote:
>> Hi. This is a rebased and mildly cleaned up version of Claudio's
>> RFC patchset from last year to add libvixl to QEMU and use it
>> for A64 disassembly.
>>
>> NOTE NOTE NOTE
>>  * we now link with g++, not gcc (even if the target doesn't
>>    happen to need the A64 disassembler, since it's a bit hard
>>    to tell whether there's a C++-source .o file in the link)
>>  * I've tested Linux (including static link of linux-user) and
>>    MacOS hosts, but not Windows
>>  * if you have a visceral dislike of the idea of C++ in the
>>    QEMU binary now would be a good time to say OMG WTF BBQ
>>
>>
>> Changes v1 -> v2:
>>  * fixed minor libvixl bugs that meant it didn't build on 32 bit
>>    hosts or on Windows
>>  * only import the files we need from libvixl, rather than
>>    pulling in 10000 lines of code we never even compile
>>  * merge aarch64-cxx.cc and aarch64.c into one file
>>  * rename 'aarch64' to 'A64' since the instruction set we're
>>    disassembling here is named A64, not AArch64
>>  * tidied the makefiles a little so we only apply the libvixl
>>    specific flags to those files
>>
>> Changes RFC -> v1:
>>  * some support for C++ compilation is already in master, so
>>    the rules.mak changes are smaller and simpler
>>  * I've fixed the output to better fit in a qemu debug log trace
>>  * I simplified the interface between aarch64-cxx.cc and aarch64.c
>>    a little bit
>>  * correctly handle the "no C++ compiler, so no A64 disassembler"
>>    build case
>>  * added support for using this as a disassembler for A64 targets
>>    as well as hosts
>>
>> This patchset sits on top of target-arm.next.
>> You can find a git tree here:
>>  git://git.linaro.org/people/peter.maydell/qemu-arm.git a64-vixl
>> web UI:
>>  
>> https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/a64-vixl
>>
>>
>> Claudio Fontana (1):
>>   disas: Implement disassembly output for A64
>>
>> Peter Maydell (4):
>>   rules.mak: Support .cc as a C++ source file suffix
>>   rules.mak: Link with C++ if we have a C++ compiler
>>   disas: Add subset of libvixl sources for A64 disassembler
>>   disas/libvixl: Fix upstream libvixl compilation issues
>>
>>  configure                             |    4 +
>>  disas.c                               |   14 +-
>>  disas/Makefile.objs                   |    5 +
>>  disas/arm-a64.cc                      |   87 ++
>>  disas/libvixl/LICENCE                 |   30 +
>>  disas/libvixl/Makefile.objs           |    8 +
>>  disas/libvixl/a64/assembler-a64.h     | 1784 
>> +++++++++++++++++++++++++++++++++
>>  disas/libvixl/a64/constants-a64.h     | 1104 ++++++++++++++++++++
>>  disas/libvixl/a64/cpu-a64.h           |   56 ++
>>  disas/libvixl/a64/decoder-a64.cc      |  712 +++++++++++++
>>  disas/libvixl/a64/decoder-a64.h       |  198 ++++
>>  disas/libvixl/a64/disasm-a64.cc       | 1678 +++++++++++++++++++++++++++++++
>>  disas/libvixl/a64/disasm-a64.h        |  109 ++
>>  disas/libvixl/a64/instructions-a64.cc |  238 +++++
>>  disas/libvixl/a64/instructions-a64.h  |  344 +++++++
>>  disas/libvixl/globals.h               |   65 ++
>>  disas/libvixl/platform.h              |   43 +
>>  disas/libvixl/utils.cc                |  120 +++
>>  disas/libvixl/utils.h                 |  126 +++
>>  include/disas/bfd.h                   |    1 +
>>  rules.mak                             |   14 +-
>>  target-arm/translate-a64.c            |    2 +-
>>  22 files changed, 6736 insertions(+), 6 deletions(-)
>>  create mode 100644 disas/arm-a64.cc
>>  create mode 100644 disas/libvixl/LICENCE
>>  create mode 100644 disas/libvixl/Makefile.objs
>>  create mode 100644 disas/libvixl/a64/assembler-a64.h
>>  create mode 100644 disas/libvixl/a64/constants-a64.h
>>  create mode 100644 disas/libvixl/a64/cpu-a64.h
>>  create mode 100644 disas/libvixl/a64/decoder-a64.cc
>>  create mode 100644 disas/libvixl/a64/decoder-a64.h
>>  create mode 100644 disas/libvixl/a64/disasm-a64.cc
>>  create mode 100644 disas/libvixl/a64/disasm-a64.h
>>  create mode 100644 disas/libvixl/a64/instructions-a64.cc
>>  create mode 100644 disas/libvixl/a64/instructions-a64.h
>>  create mode 100644 disas/libvixl/globals.h
>>  create mode 100644 disas/libvixl/platform.h
>>  create mode 100644 disas/libvixl/utils.cc
>>  create mode 100644 disas/libvixl/utils.h
>>
>> --
>> 1.8.5
>