RE: [PATCH 1/2] Add support for O32 FPXX ABI

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

While testing the GCC patch for O32 FPXX I (luckily) hit a few issues and I
think that the way in which the FP ABI is inferred in GAS needs to be different
from the patch I posted. We've discussed this topic before and had some
differing opinions so I'll explain my current thinking and see what you think.

The problem I hit when testing was that I found some assembler modules that were
marked as FP32 even when built with -mfpxx. This was down to a specs bug in my
gcc patch that was not passing on -mfpxx to the assembler but this was good as
it turns out. If -mfpxx had been passed then the modules would have silently
been marked as FPXX and I wouldn't have found that this was wrong until runtime
which would be unpleasant to say the least. I therefore think that GAS should
only infer the default FP ABI unless explicitly told that it should infer a
different one. I've annotated the offending code below from tc-mips.c:

+  /* Only update the ABI if it is not already specified.  A .gnu_attribute
+     always wins.  */
+  if (fpabi == Val_GNU_MIPS_ABI_FP_ANY)
+    {

This remains important. We do not want to clobber an explicit .gnu_attribute.

+      /* Soft-float gets precedence over single-float, the two options should
+	 not be used together so this should not matter.  */
+      if (file_mips_opts.soft_float == 1)
+	fpabi = Val_GNU_MIPS_ABI_FP_SOFT;

This is OK since -msoft-float guarantees no floating point instructions are
enabled

+      /* If floating-point code has been seen and the module is single-float
+	 then give this precedence over the double-precision cases below.
+	 Single-float can theoretically be used with any width register.  */
+      else if (mips_seen_fp_code == TRUE
+	       && file_mips_opts.single_float == 1)
+	fpabi = Val_GNU_MIPS_ABI_FP_SINGLE;

This is not OK. As we don't know if the user wrote .module singlefloat or used
-msingle-float. If it was .module then this shows intent that the code in this
module is designed for single-float so that would be OK.

+      else if (mips_seen_fp_code == TRUE)
+	{
+	  switch (file_mips_opts.fp)
+	    {
+	    case 32:
+	      fpabi = Val_GNU_MIPS_ABI_FP_DOUBLE;

This is OK. It should be safe to say that all hand-crafted modules using 32-bit
floating point registers comply with the default double precision ABI.

+	      break;
+	    case 0:
+	      fpabi = Val_GNU_MIPS_ABI_FP_XX;

This is not OK. We don't know if it was .module fp=xx or -mfpxx. Same rationale
as single-float.

+	      break;
+	    case 64:
+	      if (file_mips_opts.gp32)
+		fpabi = Val_GNU_MIPS_ABI_FP_64;

This is not OK. We don't know if it was .module fp=64 or -mfp64. Same rationale
as single-float.

+	      else
+		fpabi = Val_GNU_MIPS_ABI_FP_DOUBLE;

This is OK. Same rationale as "case 32" above but for 64-bit ABIs.

+	      break;
+	    }
+	}
+
+      bfd_elf_add_obj_attr_int (stdoutput, OBJ_ATTR_GNU,
+				Tag_GNU_MIPS_ABI_FP, fpabi);
+    }

I propose a further option to fix this. -minfer-abi and .module infer-fpabi. The
updated code would look like this and will force assembly code writers who use
floating-point to think about the ABI or stick with the default.

  /* Only update the ABI if it is not already specified.  A .gnu_attribute
     always wins.  */
  if (fpabi == Val_GNU_MIPS_ABI_FP_ANY)
    {
      /* Soft-float gets precedence over single-float, the two options should
         not be used together so this should not matter.  */
      if (file_mips_opts.soft_float == 1)
        fpabi = Val_GNU_MIPS_ABI_FP_SOFT;
      else if (mips_seen_fp_code == TRUE)
        {
          if (!mips_infer_fpabi)
            fpabi = Val_GNU_MIPS_ABI_FP_DOUBLE;
          else
            {
              /* If floating-point code has been seen and the module is
                 single-float then give this precedence over the
                 double-precision cases below.  Single-float can theoretically
                 be used with any width register.  */
              if (file_mips_opts.single_float == 1)
                fpabi = Val_GNU_MIPS_ABI_FP_SINGLE;
              else
                switch (file_mips_opts.fp)
                  {
                  case 32:
                    fpabi = Val_GNU_MIPS_ABI_FP_DOUBLE;
                    break;
                  case 0:
                    fpabi = Val_GNU_MIPS_ABI_FP_XX;
                    break;
                  case 64:
                    if (file_mips_opts.gp32)
                      fpabi = Val_GNU_MIPS_ABI_FP_64;
                    else
                      fpabi = Val_GNU_MIPS_ABI_FP_DOUBLE;
                    break;
                  }
            }
        }

      bfd_elf_add_obj_attr_int (stdoutput, OBJ_ATTR_GNU,
                                Tag_GNU_MIPS_ABI_FP, fpabi);
    }

Regards,
Matthew