Reducing the significance of -mips16

[Richard Sandiford <rdsandiford at googlemail dot com>]

One gripe I had with the (generally very good) implementation of
MIPS16 attributes was that mips_override_options continued to select
the command-line setting of -mips16.  This means that compiling with
-mips16 is subtly different from forcing every function to be MIPS16,
even ignoring the differences in compiler-generated functions.

Now that David has implemented atomic operations (thanks!),
the difference is very important.  As it stands, the
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros are defined when compiling
with -mno-mips16 but not when compiling with -mips16.  The problem
is that headers like <ext/concurrence.h> rely on these macros,
so compiling with -mips16 can unintentionally change the ABI.

A naive suggestion of mine to "fix" this aspect of the implementation
didn't work, for reasons I can't remember.  (I think the discussion was
internal to CS, so I don't have access to it anymore.)  However, I do
remember we hit PCH problems at one stage, and the naive suggestion
still does.  So perhaps it was that.

The testsuite actually showed up two separate PCH problems, both of
which I think are target_reinit bugs.  I've just posted patches
for approval.

This patch therefore reinstates the naive suggestion, which was to
treat all compilations as -mno-mips16 to begin with, and to only
switch to MIPS16 mode when we start generating code.  (There will
be no further switches after that unless the TU uses "nomips16"
attributes.)

Also, it is unsafe to use a single GTY(())ed was_mips16_p, because much
of the information calculated by target_reinit is not itself GTY(())ed.
(The information that isn't GTY(())ed is determined by target flags,
which must be the same for PCH generation and use.)

Thus if the compilation of a PCH ended in the opposite mode from the
command-line flag, loading the PCH would wrongly lead us to believe that
we had fully switched modes.  This bug was masking the target_reinit ones.

To be fully safe, we need two flags: a GTY(())ed version and a
non-GTY(())ed version.  We must go through a full target_reinit
if the two flags differ.

I suppose the ultra-paranoid (and thus more robust) approach would
be to do this switch immediately after restoring the GC state.
However, as it stands, I believe we can postpone the switch until
the next natural call to mips_set_mips16_mode.  (In particular,
cfun itself isn't -- and doesn't need to be -- GTY(())ed.)
I'll revisit this after function-specific-branch is merged.

Tested on mipsisa64-elfoabi, including -mflip-mips16 tests[*].
I'll commit if there are no objections, and if the prerequisite
patches are approved.

  [*] Although the -mflip-mips16 results are awful for unrelated reasons.
      pch.exp and mips.exp are still clean.

Richard


gcc/
	* config/mips/mips.h (TARGET_CPU_CPP_BUILTINS): Check
	mips_base_mips16 instead of TARGET_MIPS16.
	(mips_base_mips16): Declare.
	* config/mips/mips.c (mips_base_mips16): Make global.
	(was_mips16_p): Remove GTY marker.
	(was_mips16_pch_p): New variable.
	(mips_set_mips16_mode): Check both was_mips16_p and was_mips16_pch_p.
	(mips_override_options): Force to non-MIPS16 mode initially.
	Do not complain about MIPS16 PIC incompatibilities here.
	Only allow -mgpopt if -mexplicit-relocs is in force for
	non-MIPS16 code.

gcc/testsuite/
	* gcc.target/mips/gcc-have-sync-compare-and-swap-1.c: Expect the
	macros to be defined for MIPS16 too.
	* gcc.target/mips/gcc-have-sync-compare-and-swap-2.c: Likewise.
	* gcc.target/mips/gcc-have-sync-compare-and-swap-3.c: New test.
	* gcc.target/mips/gcc-have-sync-compare-and-swap-4.c: Likewise.

Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	2008-06-26 21:51:02.000000000 +0100
+++ gcc/config/mips/mips.h	2008-06-26 22:19:15.000000000 +0100
@@ -390,7 +390,7 @@ enum mips_code_readable_setting {
       else								\
 	builtin_define ("__mips_fpr=32");				\
 									\
-      if (TARGET_MIPS16)						\
+      if (mips_base_mips16)						\
 	builtin_define ("__mips16");					\
 									\
       if (TARGET_MIPS3D)						\
@@ -3227,6 +3227,7 @@ extern int mips_abi;			/* which ABI to u
 extern const struct mips_cpu_info *mips_arch_info;
 extern const struct mips_cpu_info *mips_tune_info;
 extern const struct mips_rtx_cost_data *mips_cost;
+extern bool mips_base_mips16;
 extern enum mips_code_readable_setting mips_code_readable;
 #endif
 
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2008-06-26 21:50:57.000000000 +0100
+++ gcc/config/mips/mips.c	2008-06-26 22:19:18.000000000 +0100
@@ -441,7 +441,7 @@ const struct mips_rtx_cost_data *mips_co
 static int mips_base_target_flags;
 
 /* True if MIPS16 is the default mode.  */
-static bool mips_base_mips16;
+bool mips_base_mips16;
 
 /* The ambient values of other global variables.  */
 static int mips_base_delayed_branch; /* flag_delayed_branch */
@@ -12289,8 +12289,14 @@ mips_output_mi_thunk (FILE *file, tree t
 }
 
 /* The last argument passed to mips_set_mips16_mode, or negative if the
-   function hasn't been called yet.  */
-static GTY(()) int was_mips16_p = -1;
+   function hasn't been called yet.
+
+   There are two copies of this information.  One is saved and restored
+   by the PCH process while the other is specific to this compiler
+   invocation.  The information calculated by mips_set_mips16_mode
+   is invalid unless the two variables are the same.  */
+static int was_mips16_p = -1;
+static GTY(()) int was_mips16_pch_p = -1;
 
 /* Set up the target-dependent global state so that it matches the
    current function's ISA mode.  */
@@ -12298,7 +12304,8 @@ static GTY(()) int was_mips16_p = -1;
 static void
 mips_set_mips16_mode (int mips16_p)
 {
-  if (mips16_p == was_mips16_p)
+  if (mips16_p == was_mips16_p
+      && mips16_p == was_mips16_pch_p)
     return;
 
   /* Restore base settings of various flags.  */
@@ -12373,11 +12380,12 @@ mips_set_mips16_mode (int mips16_p)
   /* (Re)initialize MIPS target internals for new ISA.  */
   mips_init_relocs ();
 
-  if (was_mips16_p >= 0)
+  if (was_mips16_p >= 0 || was_mips16_pch_p >= 0)
     /* Reinitialize target-dependent state.  */
     target_reinit ();
 
   was_mips16_p = mips16_p;
+  was_mips16_pch_p = mips16_p;
 }
 
 /* Implement TARGET_SET_CURRENT_FUNCTION.  Decide whether the current
@@ -12583,6 +12591,10 @@ mips_override_options (void)
 {
   int i, start, regno, mode;
 
+  /* Process flags as though we were generating non-MIPS16 code.  */
+  mips_base_mips16 = TARGET_MIPS16;
+  target_flags &= ~MASK_MIPS16;
+
 #ifdef SUBTARGET_OVERRIDE_OPTIONS
   SUBTARGET_OVERRIDE_OPTIONS;
 #endif
@@ -12728,14 +12740,6 @@ mips_override_options (void)
       target_flags &= ~MASK_ABICALLS;
     }
 
-  /* MIPS16 cannot generate PIC yet.  */
-  if (TARGET_MIPS16 && (flag_pic || TARGET_ABICALLS))
-    {
-      sorry ("MIPS16 PIC");
-      target_flags &= ~MASK_ABICALLS;
-      flag_pic = flag_pie = flag_shlib = 0;
-    }
-
   if (TARGET_ABICALLS)
     /* We need to set flag_pic for executables as well as DSOs
        because we may reference symbols that are not defined in
@@ -12763,7 +12767,7 @@ mips_override_options (void)
     {
       if (!TARGET_GPOPT)
 	{
-	  if (!TARGET_MIPS16 && !TARGET_EXPLICIT_RELOCS)
+	  if (!TARGET_EXPLICIT_RELOCS)
 	    error ("%<-mno-gpopt%> needs %<-mexplicit-relocs%>");
 
 	  TARGET_LOCAL_SDATA = false;
@@ -12867,7 +12871,6 @@ mips_override_options (void)
     target_flags |= MASK_FIX_R4400;
 
   /* Save base state of options.  */
-  mips_base_mips16 = TARGET_MIPS16;
   mips_base_target_flags = target_flags;
   mips_base_delayed_branch = flag_delayed_branch;
   mips_base_schedule_insns = flag_schedule_insns;
@@ -12877,8 +12880,11 @@ mips_override_options (void)
   mips_base_align_jumps = align_jumps;
   mips_base_align_functions = align_functions;
 
-  /* Now select the ISA mode.  */
-  mips_set_mips16_mode (mips_base_mips16);
+  /* Now select the ISA mode.
+
+     Do all CPP-sensitive stuff in non-MIPS16 mode; we'll switch to
+     MIPS16 mode afterwards if need be.  */
+  mips_set_mips16_mode (false);
 
   /* We call dbr_schedule from within mips_reorg.  */
   flag_delayed_branch = 0;
Index: gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-1.c	2008-06-26 21:50:57.000000000 +0100
+++ gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-1.c	2008-06-26 21:51:03.000000000 +0100
@@ -1,15 +1,15 @@
 /* { dg-do preprocess } */
 /* { dg-mips-options "-mips2" } */
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
 #error nonono
 #endif
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
 #error nonono
 #endif
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
 #error nonono
 #endif
 
Index: gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-2.c
===================================================================
--- gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-2.c	2008-06-26 21:50:57.000000000 +0100
+++ gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-2.c	2008-06-26 21:51:03.000000000 +0100
@@ -1,19 +1,19 @@
 /* { dg-do preprocess } */
 /* { dg-mips-options "-mgp64" } */
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
 #error nonono
 #endif
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
 #error nonono
 #endif
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
 #error nonono
 #endif
 
-#if defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8) == defined (__mips16)
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
 #error nonono
 #endif
 
Index: gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-3.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-3.c	2008-06-26 21:51:03.000000000 +0100
@@ -0,0 +1,23 @@
+/* { dg-do preprocess { target mips16_attribute } } */
+/* { dg-mips-options "-mips2 -mips16" } */
+/* { dg-add-options mips16_attribute } */
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
+#error nonono
+#endif
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
+#error nonono
+#endif
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+#error nonono
+#endif
+
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
+#error nonono
+#endif
+
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
+#error nonono
+#endif
Index: gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-4.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/gcc-have-sync-compare-and-swap-4.c	2008-06-26 21:51:03.000000000 +0100
@@ -0,0 +1,23 @@
+/* { dg-do preprocess { target mips16_attribute } } */
+/* { dg-mips-options "-mgp64 -mips16" } */
+/* { dg-add-options mips16_attribute } */
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
+#error nonono
+#endif
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
+#error nonono
+#endif
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+#error nonono
+#endif
+
+#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
+#error nonono
+#endif
+
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
+#error nonono
+#endif