[PATCH] MIPS/GAS: Disable PIC pseudo-ops in the MIPS16 mode

[PATCH] MIPS/GAS: Disable PIC pseudo-ops in the MIPS16 mode

[Maciej W. Rozycki]

Hi,

 We don't really support the PIC pseudo-ops in the MIPS16 mode, and 
frankly I have doubts if they make sense in that mode in the first place 
-- we just don't have the complementing infrastructure, e.g. the MIPS16 
counterpart of the PIC JAL macro and the limitations of the instruction 
set likely make it infeasible.

 Worse yet, some of these pseudo-ops trigger assertion failures, e.g.:

$ cat cpload16.s
	.set	mips16
	.set	noreorder
foo:
	.cpload	$25
$ mips-linux-gnu-as -KPIC cpload16.s
cpload16.s: Assembler messages:
cpload16.s:4: Internal error!
Assertion failure in macro_build_lui at [...]/gas/config/tc-mips.c line 5284.
Please report this bug.
$

so let's just disable them all altogether.

 Regression-tested successfully, for the mips-linux-gnu and mips-sde-elf 
targets.  OK to apply?

2011-10-24  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (s_cpload, s_cpsetup): Fail if MIPS16 mode.
	(s_cplocal, s_cprestore, s_cpreturn): Likewise.

  Maciej

binutils-gas-cpmips16-fix.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2011-10-24 22:15:38.555969862 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2011-10-24 22:15:38.735905037 +0100
@@ -16315,6 +16315,9 @@ s_cpload (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    as_bad (_("%s not supported in MIPS16 mode"), ".cpload");
+
   /* .cpload should be in a .set noreorder section.  */
   if (mips_opts.noreorder == 0)
     as_warn (_(".cpload not in noreorder section"));
@@ -16381,6 +16384,9 @@ s_cpsetup (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    as_bad (_("%s not supported in MIPS16 mode"), ".cpsetup");
+
   reg1 = tc_get_register (0);
   SKIP_WHITESPACE ();
   if (*input_line_pointer != ',')
@@ -16473,6 +16479,9 @@ s_cplocal (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    as_bad (_("%s not supported in MIPS16 mode"), ".cplocal");
+
   mips_gp_register = tc_get_register (0);
   demand_empty_rest_of_line ();
 }
@@ -16494,6 +16503,9 @@ s_cprestore (int ignore ATTRIBUTE_UNUSED
       return;
     }
 
+  if (mips_opts.mips16)
+    as_bad (_("%s not supported in MIPS16 mode"), ".cprestore");
+
   mips_cprestore_offset = get_absolute_expression ();
   mips_cprestore_valid = 1;
 
@@ -16530,6 +16542,9 @@ s_cpreturn (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    as_bad (_("%s not supported in MIPS16 mode"), ".cpreturn");
+
   macro_start ();
   if (mips_cpreturn_register == -1)
     {

Re: [PATCH] MIPS/GAS: Disable PIC pseudo-ops in the MIPS16 mode

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> 2011-10-24  Maciej W. Rozycki  <macro@codesourcery.com>
>
> 	gas/
> 	* config/tc-mips.c (s_cpload, s_cpsetup): Fail if MIPS16 mode.
> 	(s_cplocal, s_cprestore, s_cpreturn): Likewise.

I think these as_bad()s should be followed by a "return;".  OK with
that change.

Richard

Re: [PATCH] MIPS/GAS: Disable PIC pseudo-ops in the MIPS16 mode

[Maciej W. Rozycki]

On Tue, 15 Nov 2011, Richard Sandiford wrote:

> > 	gas/
> > 	* config/tc-mips.c (s_cpload, s_cpsetup): Fail if MIPS16 mode.
> > 	(s_cplocal, s_cprestore, s_cpreturn): Likewise.
> 
> I think these as_bad()s should be followed by a "return;".  OK with
> that change.

 That results in e.g.:

$ cat cpload16.s
	.set	mips16
	.set	noreorder
foo:
	.cpload	$25
$ mips-linux-gnu-as -KPIC cpload16.s
cpload16.s: Assembler messages:
cpload16.s:4: Error: .cpload not supported in MIPS16 mode
cpload16.s:4: Error: unknown opcode `$25'
$ 

 I have updated the patch as follows, this now just discards any following 
arguments.  I think there's little sense in bending backwards and parsing 
them properly, we don't do that for some other cases either -- see 
existing code nearby (that however uses s_ignore instead for a reason 
unknown to me, and) that doesn't even bail out (also for unknown reason -- 
I find it silly as finding these macros in non-PIC or wrong-ABI code is 
usually a sign of a user error).

 No regressions in the 23 MIPS targets.  OK to apply?

2012-08-09  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (s_cpload, s_cpsetup): Fail if MIPS16 mode.
	(s_cplocal, s_cprestore, s_cpreturn): Likewise.

  Maciej

binutils-gas-cpmips16-fix.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2012-08-09 00:44:38.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2012-08-09 00:56:48.261195291 +0100
@@ -16439,6 +16439,13 @@ s_cpload (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    {
+      as_bad (_("%s not supported in MIPS16 mode"), ".cpload");
+      ignore_rest_of_line ();
+      return;
+    }
+
   /* .cpload should be in a .set noreorder section.  */
   if (mips_opts.noreorder == 0)
     as_warn (_(".cpload not in noreorder section"));
@@ -16505,6 +16512,13 @@ s_cpsetup (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    {
+      as_bad (_("%s not supported in MIPS16 mode"), ".cpsetup");
+      ignore_rest_of_line ();
+      return;
+    }
+
   reg1 = tc_get_register (0);
   SKIP_WHITESPACE ();
   if (*input_line_pointer != ',')
@@ -16597,6 +16611,13 @@ s_cplocal (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    {
+      as_bad (_("%s not supported in MIPS16 mode"), ".cplocal");
+      ignore_rest_of_line ();
+      return;
+    }
+
   mips_gp_register = tc_get_register (0);
   demand_empty_rest_of_line ();
 }
@@ -16618,6 +16639,13 @@ s_cprestore (int ignore ATTRIBUTE_UNUSED
       return;
     }
 
+  if (mips_opts.mips16)
+    {
+      as_bad (_("%s not supported in MIPS16 mode"), ".cprestore");
+      ignore_rest_of_line ();
+      return;
+    }
+
   mips_cprestore_offset = get_absolute_expression ();
   mips_cprestore_valid = 1;
 
@@ -16654,6 +16682,13 @@ s_cpreturn (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  if (mips_opts.mips16)
+    {
+      as_bad (_("%s not supported in MIPS16 mode"), ".cpreturn");
+      ignore_rest_of_line ();
+      return;
+    }
+
   macro_start ();
   if (mips_cpreturn_register == -1)
     {

Re: [PATCH] MIPS/GAS: Disable PIC pseudo-ops in the MIPS16 mode

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> 2012-08-09  Maciej W. Rozycki  <macro@codesourcery.com>
>
> 	gas/
> 	* config/tc-mips.c (s_cpload, s_cpsetup): Fail if MIPS16 mode.
> 	(s_cplocal, s_cprestore, s_cpreturn): Likewise.

OK.

Re: [PATCH] MIPS/GAS: Disable PIC pseudo-ops in the MIPS16 mode

[Maciej W. Rozycki]

On Sun, 12 Aug 2012, Richard Sandiford wrote:

> > 	* config/tc-mips.c (s_cpload, s_cpsetup): Fail if MIPS16 mode.
> > 	(s_cplocal, s_cprestore, s_cpreturn): Likewise.
> 
> OK.

 Committed, thanks.

  Maciej