Re: [Patch] [MIPS] Change opcode membership of the jalx instruction

["Maciej W. Rozycki" <macro at codesourcery dot com>]

On Sun, 23 May 2010, Richard Sandiford wrote:

> > This patch changes the opcode membership of the jalx instruction from 
> > I16 to I1.  As a result, the use of I16 becomes obsolete along with 
> > the definitions and uses of INSN_MIPS16.  A new testcase is added and 
> > an inappropriate testcase is deleted.
> 
> Hmm, this is one of those things that someone obviously did deliberately,

 Correct:

2002-07-09  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>

	* mips-dis.c (mips_isa_type): Add MIPS16 insn handling.
	* mips-opc.c (I16): New define.
	(mips_builtin_opcodes): Make jalx an I16 insn.

It was I1 from 1996 previously.

> but the reasons why are probably lost in the mists of time.  I agree the

 Correct again. :(

> current behaviour makes little sense.  Something like:
> 
>         .set      arch=vr4130
>         jalx      foo
> 
> ought to work.

 It's worse yet -- all this arrangement enforces is that there's a -mips16 
on the command line (and then ".set nomips16" in the source).  As long as 
it's there any arch will work.  Based on the ChangeLog entries and the 
patches themselves (the above and ones that followed) I think this was an 
attempt to restrict JALX to processors it is supposed to work for.

 As the MIPS16 ASE is effectively generic (e.g. some LSI Logic TinyRISC 
cores that originally implemented the ASE are MIPS I ISA plus extensions 
(another case of "almost MIPS II ISA"); I have a LSI TR4101 manual lying 
next to me to confirm although we don't seem to support this CPU in any 
way anyway), I think JALX should be indeed accepted back to MIPS I.  
Then, possibly, it should be rejected based on arch settings that imply an 
incompatible CPU.

 That is e.g. the instruction would be assembled for -march=mips1, but 
rejected for -march=r3000.  Is it worth the hassle though?

 Note that with the advent of the microMIPS ASE things get more 
complicated yet as JALX is overloaded and used to switch to the microMIPS 
mode instead.  So binding the instruction to -mips16 has become definitely 
incorrect now, although the same mention stands -- if to be disabled, then 
for some MIPS32r2 CPU that is known to support neither ASE (I can't name 
any off the head though, sorry).

  Maciej