a couple of gas MIPS ABI issues.

[cgd at broadcom dot com]

So, while reading the gas tc-mips.c code the other day, I came across
some code that doesn't seem right in the ".set mipsN" implementation.
I've also got some questions about how (err, "why" 8-) those types of
options should relate to the ABI in use.


in the code that handles .set mipsN, in s_mipsset(), it would appear
that if you say ".set mipsN" that can _change_ the ABI in use, and if
you say .set mips0 that'll restore it.

I think this is erroneous for a couple of reasons:

	* at the very least, if you're trying to restore with ".set
          mips0", should restoration also happen if you ".set push
          ... .set pop"?  Logically, you'd (OK, I'd 8-) think it
          would.

	* I'm concerned that ".set mipsN" changes the ABI at all.  i
	  mean, the sizes of addresse in your GOT, etc., aren't going
	  to change for the binary as a whole just because you said
	  ".set mipsN" right?

	  What's the _benefit_ of setting mips_abi = NO_ABI in the
	  mips3-mips5,mips64 case?  

	  Looks to me like that was a leftover artifact/incorrect
	  conversion done when getting rid of mips32_abi?

(It also looks to me like the setting of mips_fp32 and mips_gp32 in
the .set mipsN code is entirely reduntant.  The code that checks those
flags also checks the ISA reg size, since since .set mipsN also sets
the ISA...  Also, there's a duplicate as_bad() there in that code;
don't need to check default in both switches.  8-)


So, to go along with all that, I was also thinking, really, _should_
changing the GPR size at run-time cause you to change the
"HAVE_32BIT_ADDRESSES" / "HAVE_64BIT_ADDRESSES" determination?

I guess that the way it works now, (after fixing the NO_ABI issue
mentioned above) ".set mipsN" will _only_ "downgrade" your address
size (except for embedded pic 8-), except in a very strange (illegal?
insane!) case where you're compiling for 64 bit ISA yet only with
32-bit GPRs.

I'm wondering if it makes sense to make the determiniation of
HAVE_32BIT_ADDRESSES based on the command line ISA, rather than
the current ISA potentially settable by ".set mipsN".

file_mips_isa already exists to hold that info, it seems, and is used
by .set mips0.


So, I think i'd propose something like the diff below.  I've not
actually tested it -- or even tried to compile it -- and I'll be the
first to admit that I don't understand completely how this ISA
handling is _supposed_ to work in the assembler...  8-)

No ChangeLog, because I'm not encouraging anybody to commit it w/o oh,
say, thinking about it more, compiling it and maybe testing it.  8-)


chris
===================================================================
Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.88
diff -u -p -r1.88 tc-mips.c
--- tc-mips.c	2001/11/01 01:33:46	1.88
+++ tc-mips.c	2001/11/07 20:36:02
@@ -227,10 +227,11 @@ static int mips_fp32 = 0;
    || (ISA) == ISA_MIPS64            \
    )
 
-#define HAVE_32BIT_GPRS		                   \
+#define HAVE_32BIT_GPRS_ISA(isa)                   \
     (mips_gp32                                     \
      || mips_abi == O32_ABI                        \
-     || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
+     || ! ISA_HAS_64BIT_REGS (isa))
+#define HAVE_32BIT_GPRS		HAVE_32BIT_GPRS_ISA (mips_opts.isa)
 
 #define HAVE_32BIT_FPRS                            \
     (mips_fp32                                     \
@@ -247,7 +248,7 @@ static int mips_fp32 = 0;
 /* We can only have 64bit addresses if the object file format
    supports it.  */
 #define HAVE_32BIT_ADDRESSES                           \
-   (HAVE_32BIT_GPRS                                    \
+   (HAVE_32BIT_GPRS_ISA(file_mips_isa)                 \
     || ((bfd_arch_bits_per_address (stdoutput) == 32   \
          || ! HAVE_64BIT_OBJECTS)                      \
         && mips_pic != EMBEDDED_PIC))
@@ -11012,57 +11013,12 @@ s_mipsset (x)
   else if (strncmp (name, "mips", 4) == 0)
     {
       int isa;
-      static int saved_mips_gp32;
-      static int saved_mips_fp32;
-      static enum mips_abi_level saved_mips_abi;
-      static int is_saved;
 
       /* Permit the user to change the ISA on the fly.  Needless to
 	 say, misuse can cause serious problems.  */
       isa = atoi (name + 4);
       switch (isa)
       {
-      case  0:
-	mips_gp32 = saved_mips_gp32;
-	mips_fp32 = saved_mips_fp32;
-	mips_abi = saved_mips_abi;
-	is_saved = 0;
-	break;
-      case  1:
-      case  2:
-      case 32:
-	if (! is_saved)
-	  {
-	    saved_mips_gp32 = mips_gp32;
-	    saved_mips_fp32 = mips_fp32;
-	    saved_mips_abi = mips_abi;
-	  }
-	mips_gp32 = 1;
-	mips_fp32 = 1;
-	is_saved = 1;
-	break;
-      case  3:
-      case  4:
-      case  5:
-      case 64:
-	if (! is_saved)
-	  {
-	    saved_mips_gp32 = mips_gp32;
-	    saved_mips_fp32 = mips_fp32;
-	    saved_mips_abi = mips_abi;
-	  }
-	mips_gp32 = 0;
-	mips_fp32 = 0;
-	mips_abi = NO_ABI;
-	is_saved = 1;
-	break;
-      default:
-	as_bad (_("unknown ISA level"));
-	break;
-      }
-
-      switch (isa)
-      {
       case  0: mips_opts.isa = file_mips_isa;   break;
       case  1: mips_opts.isa = ISA_MIPS1;       break;
       case  2: mips_opts.isa = ISA_MIPS2;       break;
@@ -11071,7 +11027,7 @@ s_mipsset (x)
       case  5: mips_opts.isa = ISA_MIPS5;       break;
       case 32: mips_opts.isa = ISA_MIPS32;      break;
       case 64: mips_opts.isa = ISA_MIPS64;      break;
-      default: as_bad (_("unknown ISA level")); break;
+      default: as_bad (_("unknown ISA level `%s'"), name+4); break;
       }
     }
   else if (strcmp (name, "autoextend") == 0)