RE: [PATCH v2] Add support for O32 FPXX ABI

[Matthew Fortune <Matthew dot Fortune at imgtec dot com>]

> Attached is a patch that should address all the review comments so
> far. I have switched around when the gnu_attribute is added that is
> inferred from command-line/module to the start rather than end of
> assembly. The error checking remains at the end though to ensure any
> user override is still OK.

I've also come up with some safety/consistency checks that should be
in-keeping line with your thoughts on the matter. I'd like to add the
-modd-spreg/-mno-odd-spreg options to gas (similar to GCC). When
-mno-odd-spreg or odd single precision registers are not supported by
the base ISA then it becomes possible to raise errors for violating
the FPXX ABI by using mtc1/mfc1/swc1/lwc1 with odd registers.
Since we are planning to use -mno-odd-spreg by default for all
generic ISAs then this provides sufficient safety from my perspective.

The patch for this proposal is below and should apply on top of the
FPXX patch posted previously. I'm not sure whether to just rely
on GCC to pass -mno-odd-spreg based on its understanding of defaults
or also educate gas about the defaults for each arch? By adding the
same logic to gas then use of the assembler standalone will get the
same behaviour. This error checking will also weed out any bad
inline asm in GCC too.

I haven't done a changelog as I will submit this patch separately
to FPXX if it is OK and strip out the FPXX related parts.

Regards,
Matthew

---
 gas/config/tc-mips.c |   41 ++++++++++++++++++++++++++++++++++++-----
 gas/doc/as.texinfo   |    6 ++++++
 gas/doc/c-mips.texi  |    6 ++++++
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index e6ab856..2fc2a35 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -258,6 +258,10 @@ struct mips_set_options
      Changed by .set singlefloat or .set doublefloat, command-line options
      -msingle-float or -mdouble-float.  The default is false.  */
   bfd_boolean single_float;
+
+  /* True if single-precision operations on odd-numbered registers are
+     not allowed (even if supported by ISA_HAS_ODD_SINGLE_FPR).  */
+  bfd_boolean nooddspreg;
 };
 
 /* Specifies whether module level options have been checked yet.  */
@@ -276,7 +280,7 @@ static struct mips_set_options file_mips_opts =
   /* noreorder */ 0,  /* at */ ATREG, /* warn_about_macros */ 0,
   /* nomove */ 0, /* nobopt */ 0, /* noautoextend */ 0, /* insn32 */ FALSE,
   /* gp32 */ -1, /* fp */ -1, /* arch */ CPU_UNKNOWN, /* sym32 */ FALSE,
-  /* soft_float */ FALSE, /* single_float */ FALSE
+  /* soft_float */ FALSE, /* single_float */ FALSE, /* nooddspreg */ FALSE
 };
 
 /* This is similar to file_mips_opts, but for the current set of options.  */
@@ -287,7 +291,7 @@ static struct mips_set_options mips_opts =
   /* noreorder */ 0,  /* at */ ATREG, /* warn_about_macros */ 0,
   /* nomove */ 0, /* nobopt */ 0, /* noautoextend */ 0, /* insn32 */ FALSE,
   /* gp32 */ -1, /* fp */ -1, /* arch */ CPU_UNKNOWN, /* sym32 */ FALSE,
-  /* soft_float */ FALSE, /* single_float */ FALSE
+  /* soft_float */ FALSE, /* single_float */ FALSE, /* nooddspreg */ FALSE
 };
 
 /* The set of ASEs that were selected on the command line, either
@@ -1440,6 +1444,8 @@ enum options
     OPTION_NO_PDR,
     OPTION_MVXWORKS_PIC,
     OPTION_NAN,
+    OPTION_ODD_SPREG,
+    OPTION_NO_ODD_SPREG,
     OPTION_END_OF_ENUM
   };
 
@@ -1546,6 +1552,8 @@ struct option md_longopts[] =
   {"mhard-float", no_argument, NULL, OPTION_HARD_FLOAT},
   {"msingle-float", no_argument, NULL, OPTION_SINGLE_FLOAT},
   {"mdouble-float", no_argument, NULL, OPTION_DOUBLE_FLOAT},
+  {"modd-spreg", no_argument, NULL, OPTION_ODD_SPREG},
+  {"mno-odd-spreg", no_argument, NULL, OPTION_NO_ODD_SPREG},
 
   /* Strictly speaking this next option is ELF specific,
      but we allow it for other ports as well in order to
@@ -4440,7 +4448,8 @@ mips_oddfpreg_ok (const struct mips_opcode *insn, int opnum)
     /* Let a macro pass, we'll catch it later when it is expanded.  */
     return TRUE;
 
-  if (ISA_HAS_ODD_SINGLE_FPR (mips_opts.isa) || mips_opts.arch == CPU_R5900)
+  if ((ISA_HAS_ODD_SINGLE_FPR (mips_opts.isa) || mips_opts.arch == CPU_R5900)
+      && !mips_opts.nooddspreg)
     {
       /* Allow odd registers for single-precision ops.  */
       switch (insn->pinfo & (FP_S | FP_D))
@@ -4465,7 +4474,12 @@ mips_oddfpreg_ok (const struct mips_opcode *insn, int opnum)
   if ((insn->pinfo & FP_S)
       && (insn->pinfo & (INSN_COPROC_MEMORY_DELAY | INSN_STORE_MEMORY
 			 | INSN_LOAD_COPROC_DELAY | INSN_COPROC_MOVE_DELAY)))
-    return TRUE;
+    {
+      if (mips_opts.fp == 0 && mips_opts.nooddspreg)
+	as_bad (_("unsupported access to the upper half of double-precision "
+		   "registers"));
+      return TRUE;
+    }
 
   return FALSE;
 }
@@ -4692,7 +4706,12 @@ check_regno (struct mips_arg_info *arg,
       && (regno & 1) != 0
       && HAVE_32BIT_FPRS
       && !mips_oddfpreg_ok (arg->insn->insn_mo, arg->opnum))
-    as_warn (_("float register should be even, was %d"), regno);
+    {
+      if (mips_opts.fp != 0)
+	as_warn (_("float register should be even, was %d"), regno);
+      else
+	as_bad (_("float register should be even, was %d"), regno);
+    }
 
   if (type == OP_REG_CCC)
     {
@@ -14009,6 +14028,14 @@ md_parse_option (int c, char *arg)
       file_mips_opts.fp = 64;
       break;
 
+    case OPTION_ODD_SPREG:
+      file_mips_opts.nooddspreg = FALSE;
+      break;
+
+    case OPTION_NO_ODD_SPREG:
+      file_mips_opts.nooddspreg = TRUE;
+      break;
+
     case OPTION_SINGLE_FLOAT:
       file_mips_opts.single_float = 1;
       break;
@@ -15167,6 +15194,10 @@ parse_code_option (char * name)
     mips_opts.single_float = 1;
   else if (strcmp (name, "doublefloat") == 0)
     mips_opts.single_float = 0;
+  else if (strcmp (name, "nooddspreg") == 0)
+    mips_opts.nooddspreg = TRUE;
+  else if (strcmp (name, "oddspreg") == 0)
+    mips_opts.nooddspreg = FALSE;
   else if (strcmp (name, "mips16") == 0
 	   || strcmp (name, "MIPS-16") == 0)
     mips_opts.mips16 = 1;
diff --git a/gas/doc/as.texinfo b/gas/doc/as.texinfo
index 0062bc8..be7bdfb 100644
--- a/gas/doc/as.texinfo
+++ b/gas/doc/as.texinfo
@@ -400,6 +400,7 @@ gcc(1), ld(1), and the Info entries for @file{binutils} and @file{ld}.
    [@b{-non_shared}] [@b{-xgot} [@b{-mvxworks-pic}]
    [@b{-mabi}=@var{ABI}] [@b{-32}] [@b{-n32}] [@b{-64}] [@b{-mfp32}] [@b{-mgp32}]
    [@b{-mfp64}] [@b{-mgp64}] [@b{-mfpxx}]
+   [@b{-modd-spreg}] [@b{-mno-odd-spreg}]
    [@b{-march}=@var{CPU}] [@b{-mtune}=@var{CPU}] [@b{-mips1}] [@b{-mips2}]
    [@b{-mips3}] [@b{-mips4}] [@b{-mips5}] [@b{-mips32}] [@b{-mips32r2}]
    [@b{-mips32r3}] [@b{-mips32r5}] [@b{-mips64}] [@b{-mips64r2}]
@@ -1329,6 +1330,11 @@ this flag in combination with @samp{-mabi=32} enables an ABI variant
 which will operate correctly with floating-point registers which are
 32 or 64 bits wide.
 
+@item -modd-spreg
+@itemx -mno-odd-spreg
+Enable use of floating-point operations on odd-numbered single-precision
+registers when supported by the ISA.  This is the default.
+
 @item -mips16
 @itemx -no-mips16
 Generate code for the MIPS 16 processor.  This is equivalent to putting
diff --git a/gas/doc/c-mips.texi b/gas/doc/c-mips.texi
index 7c99b2d..595bd9d 100644
--- a/gas/doc/c-mips.texi
+++ b/gas/doc/c-mips.texi
@@ -135,6 +135,12 @@ only be used with MIPS II and above.
 The @code{.set fp=xx} directive allows a part of an object to be marked
 as not making assumptions about 32-bit or 64-bita FP registers.  The
 default value is restored by @code{.set fp=default}.
+
+@item -modd-spreg
+@itemx -mno-odd-spreg
+Enable use of floating-point operations on odd-numbered single-precision
+registers when supported by the ISA.  This is the default.
+
 @item -mips16
 @itemx -no-mips16
 Generate code for the MIPS 16 processor.  This is equivalent to putting
-- 
1.7.1