Re: [PATCH 1/2] opcodes/mips: use .word/.short for undefined instructions

["Maciej W. Rozycki" <macro@orcam.me.uk>]

On Fri, 6 Jan 2023, Andrew Burgess wrote:

> >  FYI, I find this questionable as `.word' (at least with the MIPS target) 
> > implies natural alignment while 32-bit microMIPS encodings, valid or not, 
> > are not.  Also, given the endianness peculiarity (analogous to the MIPS16 
> > extended encodings), I think this needs to be ".short\t0x%x, 0x%x" really, 
> > with the instruction word split into halfwords for any reasonable meaning.  
> > This is already reflected in the raw hex dump of instruction streams; the 
> > numbers printed need to match it.
> >
> >  With the naked number previously used this obviously didn't matter as it 
> > stood out without any attempt to pretend to have a meaning.  This is also 
> > the reason why I chose to keep it as it used to be since forever.
> 
> Below is an initial patch.  When I set the environment variable
> DISABLE_MATCHING then the disassembler fails to match all instructions,
> so prints .short for everything.
> 
> Right now I can't find anything where this doesn't work, but I don't
> believe that the answer is actually this simple.  Given your deeper
> knowledge of the target, could you take a look at what I have below and
> point me at some tests/configurations/whatever where this isn't going to
> be good enough?
> 
> Alternatively, if this is enough, then I'll write this up into a proper
> patch.

 Your change is probably right.  I'd have thought we have coverage for 
this in the testsuite, but perhaps we don't.

 If you try this source code (which uses a reserved 32-bit encoding in the 
microMIPS ISA):

	.module	micromips
foo:
	.insn
	.short	0x7f6e, 0x5d4c

and assemble it for both endiannesses (i.e. with `-EL' and `-EB' passed to 
GAS respectively), then I'd expect output like:

Disassembly of section .text:

00000000 <foo>:
   0:	7f6e 5d4c 	.short	0x7f6e, 0x5d4c
	...

from `objdump -d' in both cases.  If this is the case, then the change is 
right.

 You can use this example, preferably along with the change itself, for a 
testcase to place in binutils/testsuite/binutils-all/mips/.  I suggest 
using `run_dump_test_o32'/`run_dump_test_n32'/`run_dump_test_n64' all at a 
time just as with most of the preexisting test cases just to make sure all 
the three BFD backends involved handle this right.

 Let me know if you need further information.

  Maciej