Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

Hi Fredrik,

> > > Option 3: Extend the mips_opcode::membership field.
> > 
> > It's trivial to extend the field to uint64_t.
> 
> Is the membership field intended to be used? The opcodes for CLZ and CLO
> clash with the R5900 opcodes for MADD1 and MADDU1, resulting in incorrect
> disassembly of MADD1 and MADDU1. For example:
> 
>       0x70853020 madd1  a2,a0,a1  disassembles into  clz a2 or a1,a0
>       0x70853021 maddu1 a2,a0,a1  disassembles into  clo a2 or a1,a0
> 
> (CLZ and CLO are members of I32|N55, whereas MADD1 and MADDU1 are EE.)

 It looks like a disassembler bug somewhere then (maybe in your patched 
version only), because the R5900 is not supposed to match I32 (because it 
does not implement the MIPS32 ISA; it's only MIPS I aka I1 with additions 
or MIPS IV aka I4 with exclusions, or anything between with both additions 
and exclusions, with I believe MIPS III aka I3 being the closest match), 
and it is not supposed to match N55 either (because it is obviously not a 
Vr5500 processor).

 Overall this source file is clearly a modified copy of an ancient version 
of the opcode table included with the opcodes library from binutils and I 
think it would benefit from a refresh.  In particular separating an ASE 
field and adding an exclusions field, as it has been done with opcodes, 
would make it much easier to maintain this table.  The table in opcodes is 
already messy due to several exceptions to the alphabetical order (and it 
could be improved a bit I believe), but I find its QEMU version even 
messier.

 HTH,

  Maciej