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

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

[Catherine Moore]

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

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.

Does this look okay to install?

Thanks,
Catherine

2010-05-20  Catherine Moore  <clm@codesourcery.com>

	gas/
	* config/tc-mips.c (is_opcode_valid): Remove expansionp.
	(macro_build): Don't pass expansionp to is_opcode_valid.
	(mips_ip): Likewise.

	gas/testsuite/
	* gas/mips/mips-jalx-2.s: New test.
	* gas/mips/mips-jalx-2.d: New test output.
	* gas/mips/mips-no-jalx.l: Delete.
	* gas/mips/mips-no-jalx.s: Delete.
	* gas/mips/mips.exp: Add mips-jalx-2.  Delete mips-no-jalx.

	include/opcode/
	* mips.h (INSN_MIPS16): Remove.
	opcodes/
	* mips-dis.c (mips_arch_choices): Remove INSN_MIPS16 for
	CPU_MIPS32, CPU_MIPS32R2, CPU_MIPS64 and CPU_MIPS64R2.
	* mips-opc.c (I16): Remove definition.
	(mips_builtin_op): Change membership of jalx insn.

[-- Attachment #2: jalx-patch --]
[-- Type: text/plain, Size: 8481 bytes --]

Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.417
diff -u -p -r1.417 tc-mips.c
--- gas/config/tc-mips.c	25 Feb 2010 11:15:47 -0000	1.417
+++ gas/config/tc-mips.c	20 May 2010 18:09:43 -0000
@@ -1817,11 +1817,10 @@ reg_lookup (char **s, unsigned int types
 }
 
 /* Return TRUE if opcode MO is valid on the currently selected ISA and
-   architecture.  If EXPANSIONP is TRUE then this check is done while
-   expanding a macro.  Use is_opcode_valid_16 for MIPS16 opcodes.  */
+   architecture.  Use is_opcode_valid_16 for MIPS16 opcodes.  */
 
 static bfd_boolean
-is_opcode_valid (const struct mips_opcode *mo, bfd_boolean expansionp)
+is_opcode_valid (const struct mips_opcode *mo)
 {
   int isa = mips_opts.isa;
   int fp_s, fp_d;
@@ -1841,11 +1840,6 @@ is_opcode_valid (const struct mips_opcod
   if (mips_opts.ase_smartmips)
     isa |= INSN_SMARTMIPS;
 
-  /* For user code we don't check for mips_opts.mips16 since we want
-     to allow jalx if -mips16 was specified on the command line.  */
-  if (expansionp ? mips_opts.mips16 : file_ase_mips16)
-    isa |= INSN_MIPS16;
-
   /* Don't accept instructions based on the ISA if the CPU does not implement
      all the coprocessor insns. */
   if (NO_ISA_COP (mips_opts.arch)
@@ -3638,7 +3632,7 @@ macro_build (expressionS *ep, const char
 	 macros will never generate MDMX, MIPS-3D, or MT instructions.  */
       if (strcmp (fmt, mo->args) == 0
 	  && mo->pinfo != INSN_MACRO
-	  && is_opcode_valid (mo, TRUE))
+	  && is_opcode_valid (mo))
 	break;
 
       ++mo;
@@ -8776,7 +8770,7 @@ mips_ip (char *str, struct mips_cl_insn 
 
       gas_assert (strcmp (insn->name, str) == 0);
 
-      ok = is_opcode_valid (insn, FALSE);
+      ok = is_opcode_valid (insn);
       if (! ok)
 	{
 	  if (insn + 1 < &mips_opcodes[NUMOPCODES]
Index: gas/testsuite/gas/mips/mips-jalx-2.d
===================================================================
RCS file: gas/testsuite/gas/mips/mips-jalx-2.d
diff -N gas/testsuite/gas/mips/mips-jalx-2.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/mips/mips-jalx-2.d	20 May 2010 18:09:43 -0000
@@ -0,0 +1,15 @@
+#objdump: -d
+#as:
+#name: mips jalx-2
+
+.*:     file format .*
+
+Disassembly of section .text:
+
+[ 0-9a-f]+ <text_sym>:
+[ 0-9a-f]+:	74000000 	jalx	0 <.[^>]*>
+[ 0-9a-f]+:	00000000 	nop
+
+[ 0-9a-f]+ <.[^>]*>:
+[ 0-9a-f]+:	6500      	nop
+	\.\.\.
Index: gas/testsuite/gas/mips/mips-jalx-2.s
===================================================================
RCS file: gas/testsuite/gas/mips/mips-jalx-2.s
diff -N gas/testsuite/gas/mips/mips-jalx-2.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/mips/mips-jalx-2.s	20 May 2010 18:09:43 -0000
@@ -0,0 +1,10 @@
+text_sym:
+	.text
+	jalx	1f
+	
+	.set	mips16
+	.align	1
+1:	nop
+	.set	nomips16
+	.align	2
+	.fill	8
Index: gas/testsuite/gas/mips/mips-no-jalx.l
===================================================================
RCS file: gas/testsuite/gas/mips/mips-no-jalx.l
diff -N gas/testsuite/gas/mips/mips-no-jalx.l
--- gas/testsuite/gas/mips/mips-no-jalx.l	3 Oct 2002 23:53:09 -0000	1.2
+++ /dev/null	1 Jan 1970 00:00:00 -0000
@@ -1,2 +0,0 @@
-.*: Assembler messages:
-.*:3: Error: opcode not supported (at this ISA level|on this processor: .*) \(mips.*\) `jalx external_label'
Index: gas/testsuite/gas/mips/mips-no-jalx.s
===================================================================
RCS file: gas/testsuite/gas/mips/mips-no-jalx.s
diff -N gas/testsuite/gas/mips/mips-no-jalx.s
--- gas/testsuite/gas/mips/mips-no-jalx.s	26 Sep 2002 09:56:33 -0000	1.1
+++ /dev/null	1 Jan 1970 00:00:00 -0000
@@ -1,3 +0,0 @@
-# Test the generation of jalx opcodes
-	.set nomips16
-	jalx	external_label
Index: gas/testsuite/gas/mips/mips.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips.exp,v
retrieving revision 1.155
diff -u -p -r1.155 mips.exp
--- gas/testsuite/gas/mips/mips.exp	25 Feb 2010 11:15:48 -0000	1.155
+++ gas/testsuite/gas/mips/mips.exp	20 May 2010 18:09:43 -0000
@@ -526,6 +526,7 @@ if { [istarget mips*-*-vxworks*] } {
 	# Check jalx handling
 	run_dump_test "mips16-jalx"
 	run_dump_test "mips-jalx"
+	run_dump_test "mips-jalx-2"
 	# Check MIPS16 HI16/LO16 relocations
 	run_dump_test "mips16-hilo"
 	if $has_newabi {
@@ -533,7 +534,6 @@ if { [istarget mips*-*-vxworks*] } {
 	}
 	run_dump_test "mips16-hilo-match"
     }
-    run_list_test "mips-no-jalx" "-32"
     run_dump_test "delay"
     run_dump_test "nodelay"
     run_dump_test "mips4010"
Index: include/opcode/mips.h
===================================================================
RCS file: /cvs/src/src/include/opcode/mips.h,v
retrieving revision 1.65
diff -u -p -r1.65 mips.h
--- include/opcode/mips.h	15 Apr 2010 10:26:09 -0000	1.65
+++ include/opcode/mips.h	20 May 2010 18:09:43 -0000
@@ -555,8 +555,7 @@ static const unsigned int mips_isa_table
 /* DSP ASE */ 
 #define INSN_DSP                  0x00001000
 #define INSN_DSP64                0x00002000
-/* MIPS 16 ASE */
-#define INSN_MIPS16               0x00004000
+
 /* MIPS-3D ASE */
 #define INSN_MIPS3D               0x00008000
 
Index: opcodes/mips-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/mips-dis.c,v
retrieving revision 1.78
diff -u -p -r1.78 mips-dis.c
--- opcodes/mips-dis.c	2 Sep 2009 07:20:30 -0000	1.78
+++ opcodes/mips-dis.c	20 May 2010 18:09:43 -0000
@@ -472,13 +472,13 @@ const struct mips_arch_choice mips_arch_
      MIPS32 Architecture_ (MIPS Document Number MD00082, Revision 0.95),
      page 1.  */
   { "mips32",	1, bfd_mach_mipsisa32, CPU_MIPS32,
-    ISA_MIPS32 | INSN_MIPS16 | INSN_SMARTMIPS,
+    ISA_MIPS32 | INSN_SMARTMIPS,
     mips_cp0_names_mips3264,
     mips_cp0sel_names_mips3264, ARRAY_SIZE (mips_cp0sel_names_mips3264),
     mips_hwr_names_numeric },
 
   { "mips32r2",	1, bfd_mach_mipsisa32r2, CPU_MIPS32R2,
-    (ISA_MIPS32R2 | INSN_MIPS16 | INSN_SMARTMIPS | INSN_DSP | INSN_DSPR2
+    (ISA_MIPS32R2 | INSN_SMARTMIPS | INSN_DSP | INSN_DSPR2
      | INSN_MIPS3D | INSN_MT),
     mips_cp0_names_mips3264r2,
     mips_cp0sel_names_mips3264r2, ARRAY_SIZE (mips_cp0sel_names_mips3264r2),
@@ -486,13 +486,13 @@ const struct mips_arch_choice mips_arch_
 
   /* For stock MIPS64, disassemble all applicable MIPS-specified ASEs.  */
   { "mips64",	1, bfd_mach_mipsisa64, CPU_MIPS64,
-    ISA_MIPS64 | INSN_MIPS16 | INSN_MIPS3D | INSN_MDMX,
+    ISA_MIPS64 | INSN_MIPS3D | INSN_MDMX,
     mips_cp0_names_mips3264,
     mips_cp0sel_names_mips3264, ARRAY_SIZE (mips_cp0sel_names_mips3264),
     mips_hwr_names_numeric },
 
   { "mips64r2",	1, bfd_mach_mipsisa64r2, CPU_MIPS64R2,
-    (ISA_MIPS64R2 | INSN_MIPS16 | INSN_MIPS3D | INSN_DSP | INSN_DSPR2
+    (ISA_MIPS64R2 | INSN_MIPS3D | INSN_DSP | INSN_DSPR2
      | INSN_DSP64 | INSN_MT | INSN_MDMX),
     mips_cp0_names_mips3264r2,
     mips_cp0sel_names_mips3264r2, ARRAY_SIZE (mips_cp0sel_names_mips3264r2),
@@ -524,7 +524,7 @@ const struct mips_arch_choice mips_arch_
 
   /* This entry, mips16, is here only for ISA/processor selection; do
      not print its name.  */
-  { "",		1, bfd_mach_mips16, CPU_MIPS16, ISA_MIPS3 | INSN_MIPS16,
+  { "",		1, bfd_mach_mips16, CPU_MIPS16, ISA_MIPS3,
     mips_cp0_names_numeric, NULL, 0, mips_hwr_names_numeric },
 };
 
Index: opcodes/mips-opc.c
===================================================================
RCS file: /cvs/src/src/opcodes/mips-opc.c,v
retrieving revision 1.75
diff -u -p -r1.75 mips-opc.c
--- opcodes/mips-opc.c	2 Sep 2009 07:20:30 -0000	1.75
+++ opcodes/mips-opc.c	20 May 2010 18:09:43 -0000
@@ -96,9 +96,6 @@
 #define I4_33   INSN_ISA4_32R2
 #define I5_33   INSN_ISA5_32R2
 
-/* MIPS16 ASE support.  */
-#define I16     INSN_MIPS16
-
 /* MIPS64 MIPS-3D ASE support.  */
 #define M3D     INSN_MIPS3D
 
@@ -739,7 +736,7 @@ const struct mips_opcode mips_builtin_op
    assembler, but will never match user input (because the line above
    will match first).  */
 {"jal",     "a",	0x0c000000, 0xfc000000,	UBD|WR_31,		0,		I1	},
-{"jalx",    "a",	0x74000000, 0xfc000000, UBD|WR_31,		0,		I16     },
+{"jalx",    "a",	0x74000000, 0xfc000000, UBD|WR_31,		0,		I1	},
 {"la",      "t,A(b)",	0,    (int) M_LA_AB,	INSN_MACRO,		0,		I1	},
 {"lb",      "t,o(b)",	0x80000000, 0xfc000000,	LDD|RD_b|WR_t,		0,		I1	},
 {"lb",      "t,A(b)",	0,    (int) M_LB_AB,	INSN_MACRO,		0,		I1	},

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

[Richard Sandiford]

Catherine Moore <clm@codesourcery.com> writes:
> 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,
but the reasons why are probably lost in the mists of time.  I agree the
current behaviour makes little sense.  Something like:

        .set      arch=vr4130
        jalx      foo

ought to work.

> 	gas/
> 	* config/tc-mips.c (is_opcode_valid): Remove expansionp.
> 	(macro_build): Don't pass expansionp to is_opcode_valid.
> 	(mips_ip): Likewise.
>
> 	gas/testsuite/
> 	* gas/mips/mips-jalx-2.s: New test.
> 	* gas/mips/mips-jalx-2.d: New test output.
> 	* gas/mips/mips-no-jalx.l: Delete.
> 	* gas/mips/mips-no-jalx.s: Delete.
> 	* gas/mips/mips.exp: Add mips-jalx-2.  Delete mips-no-jalx.
>
> 	include/opcode/
> 	* mips.h (INSN_MIPS16): Remove.
> 	opcodes/
> 	* mips-dis.c (mips_arch_choices): Remove INSN_MIPS16 for
> 	CPU_MIPS32, CPU_MIPS32R2, CPU_MIPS64 and CPU_MIPS64R2.
> 	* mips-opc.c (I16): Remove definition.
> 	(mips_builtin_op): Change membership of jalx insn.

OK if no-one objects in 48 hours.

> -/* MIPS 16 ASE */
> -#define INSN_MIPS16               0x00004000
> +

Maybe leave a comment saying that 0x4000 is unused?

Richard

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

[Maciej W. Rozycki]

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