Re: [Qemu-devel] [PATCH 4/4] Addition of Cavium instruction in disassembler

[Richard Henderson]

On 08/15/2011 04:25 AM, address@hidden wrote:
> index 1334b8e..0137657 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -140,6 +140,7 @@ print_insn_thumb1(bfd_vma pc, disassemble_info *info)
>      i386 - nonzero means 16 bit code
>      arm  - nonzero means thumb code
>      ppc  - nonzero means little endian
> +    mips64 - zero means standard MIPS ISA, 1 means Octeon CPU.
...
> +++ b/target-mips/translate.c
> @@ -12860,6 +12860,9 @@ done_generating:
>      LOG_DISAS("\n");
>      if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> +        if (env->insn_flags & CPU_OCTEON)
> +            log_target_disas(pc_start, ctx.pc - pc_start, 1);
> +        else
>          log_target_disas(pc_start, ctx.pc - pc_start, 0);

disas.c has access to cpu.h.  You'd do well to avoid a magic number
here and pass along env->insn_flags right from the beginning, and...

> @@ -196,6 +197,9 @@ void target_disas(FILE *out, target_ulong code, 
> target_ulong size, int flags)
>      print_insn = print_insn_m68k;
>  #elif defined(TARGET_MIPS)
>  #ifdef TARGET_WORDS_BIGENDIAN
> +    if (flags)
> +        disasm_info.flags = flags << 16;
> +        print_insn = print_insn_big_mips;
>      print_insn = print_insn_big_mips;

Notice anything funny here in the last two lines?
Notice anything incorrect about the coding style?

It's a bit sad that we're limited to pre-GPLv3, because we're going
to get more and more out of sync with the binutils disassembler.
But that said, let's not go out of our way to totally change the style.

Give mips-dis.c access to target-mips/mips-defs.h.  Map between that
and the existing membership field in struct mips_opcode.  Put some
real code back into OPCODE_IS_MEMBER.


r~