public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks
@ 2014-09-10 20:48 Matthew Fortune
  2014-09-13  8:19 ` Richard Sandiford
  0 siblings, 1 reply; 5+ messages in thread
From: Matthew Fortune @ 2014-09-10 20:48 UTC (permalink / raw)
  To: binutils; +Cc: Richard Sandiford, Andrew Bennett

This patch fixes a subtle mistake in the FP ABI consistency check
logic.  The error reporting does not currently follow the
intended precedence of the various FP ABIs. I.e. softfloat,
singlefloat and then all the hardfloat variants.  When someone
uses -msingle-float with a GNU attribute which is not 4,2 then
the initial warning should be that it is not compatible with
singlefloat. Likewise for softfloat and attribute 4,3.

Tested on a variety of configurations including mips-linux-gnu.

Thanks,
Matthew

gas/

	* tc-mips.c (check_fpabi): Move softfloat and single float
	checks higher.

gas/testsuite/

	* gas/mips/attr-gnu-4-5-msingle-float.l: New file.
	* gas/mips/attr-gnu-4-5-msingle-float.s: Likewise.
	* gas/mips/attr-gnu-4-5-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-5-msoft-float.s: Likewise.
	* gas/mips/attr-gnu-4-6-msingle-float.l: Update expected output.
	* gas/mips/attr-gnu-4-6-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-7-msingle-float.l: Likewise.
	* gas/mips/attr-gnu-4-7-msoft-float.l: Likewise.
	* gas/mips/mips.exp: Update expected output for FP ABI 5,6,7.

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 59d8635..2dabdf4 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -3672,39 +3672,44 @@ fpabi_requires (int fpabi, const char *what)
 static void
 check_fpabi (int fpabi)
 {
-  bfd_boolean needs_check = FALSE;
   switch (fpabi)
     {
     case Val_GNU_MIPS_ABI_FP_DOUBLE:
+      if (file_mips_opts.soft_float)
+	fpabi_incompatible_with (fpabi, "softfloat");
+      else if (file_mips_opts.single_float)
+	fpabi_incompatible_with (fpabi, "singlefloat");
       if (file_mips_opts.gp == 64 && file_mips_opts.fp == 32)
 	fpabi_incompatible_with (fpabi, "gp=64 fp=32");
       else if (file_mips_opts.gp == 32 && file_mips_opts.fp == 64)
 	fpabi_incompatible_with (fpabi, "gp=32 fp=64");
-      else
-	needs_check = TRUE;
       break;
 
     case Val_GNU_MIPS_ABI_FP_XX:
       if (mips_abi != O32_ABI)
 	fpabi_requires (fpabi, "-mabi=32");
+      else if (file_mips_opts.soft_float)
+	fpabi_incompatible_with (fpabi, "softfloat");
+      else if (file_mips_opts.single_float)
+	fpabi_incompatible_with (fpabi, "singlefloat");
       else if (file_mips_opts.fp != 0)
 	fpabi_requires (fpabi, "fp=xx");
-      else
-	needs_check = TRUE;
       break;
 
     case Val_GNU_MIPS_ABI_FP_64A:
     case Val_GNU_MIPS_ABI_FP_64:
       if (mips_abi != O32_ABI)
 	fpabi_requires (fpabi, "-mabi=32");
+      else if (file_mips_opts.soft_float)
+	fpabi_incompatible_with (fpabi, "softfloat");
+      else if (file_mips_opts.single_float)
+	fpabi_incompatible_with (fpabi, "singlefloat");
       else if (file_mips_opts.fp != 64)
 	fpabi_requires (fpabi, "fp=64");
       else if (fpabi == Val_GNU_MIPS_ABI_FP_64 && !file_mips_opts.oddspreg)
 	fpabi_incompatible_with (fpabi, "nooddspreg");
       else if (fpabi == Val_GNU_MIPS_ABI_FP_64A && file_mips_opts.oddspreg)
 	fpabi_requires (fpabi, "nooddspreg");
-      else
-	needs_check = TRUE;
       break;
 
     case Val_GNU_MIPS_ABI_FP_SINGLE:
@@ -3729,11 +3734,6 @@ check_fpabi (int fpabi)
 	         " floating-point ABI"), Tag_GNU_MIPS_ABI_FP, fpabi);
       break;
     }
-
-  if (needs_check && file_mips_opts.soft_float)
-    fpabi_incompatible_with (fpabi, "softfloat");
-  else if (needs_check && file_mips_opts.single_float)
-    fpabi_incompatible_with (fpabi, "singlefloat");
 }
 
 /* Perform consistency checks on the current options.  */
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l
new file mode 100644
index 0000000..be2017d
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*: Warning: .gnu_attribute 4,5 is incompatible with `singlefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s
new file mode 100644
index 0000000..b21ec3b
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s
@@ -0,0 +1 @@
+.gnu_attribute 4,5
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l
new file mode 100644
index 0000000..3385c5f
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*: Warning: .gnu_attribute 4,5 is incompatible with `softfloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s
new file mode 100644
index 0000000..b21ec3b
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s
@@ -0,0 +1 @@
+.gnu_attribute 4,5
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l
index 999b5e2..ab12f1d 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 is incompatible with `singlefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l
index 999b5e2..3bf1946 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 is incompatible with `softfloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l
index 999b5e2..ab12f1d 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 is incompatible with `singlefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l
index 999b5e2..3bf1946 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 is incompatible with `softfloat'
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index 8f36918..1495b9b 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -1325,9 +1325,9 @@ if { [istarget mips*-*-vxworks*] } {
 				    [mips_arch_list_matching mips32r2]
     run_list_test_arches "attr-gnu-4-5-64" "-64 -mfp64" \
 				    [mips_arch_list_matching mips3]
-    run_list_test_arches "attr-gnu-4-5" "-32 -msingle-float" \
+    run_list_test_arches "attr-gnu-4-5-msingle-float" "-32 -msingle-float" \
 				    [mips_arch_list_matching mips1]
-    run_list_test_arches "attr-gnu-4-5" "-32 -msoft-float" \
+    run_list_test_arches "attr-gnu-4-5-msoft-float" "-32 -msoft-float" \
 				    [mips_arch_list_matching mips1]
     run_dump_test_arches "attr-gnu-4-5" \
 				    [mips_arch_list_matching mips2 !r5900]
@@ -1338,9 +1338,9 @@ if { [istarget mips*-*-vxworks*] } {
 				    [mips_arch_list_matching mips32r2]
     run_list_test_arches "attr-gnu-4-6-64" "-64 -mfp64" \
 				    [mips_arch_list_matching mips3]
-    run_list_test_arches "attr-gnu-4-6" "-32 -msingle-float" \
+    run_list_test_arches "attr-gnu-4-6-msingle-float" "-32 -msingle-float" \
 				    [mips_arch_list_matching mips1]
-    run_list_test_arches "attr-gnu-4-6" "-32 -msoft-float" \
+    run_list_test_arches "attr-gnu-4-6-msoft-float" "-32 -msoft-float" \
 				    [mips_arch_list_matching mips1]
     run_list_test_arches "attr-gnu-4-6" "-32 -mfpxx" \
 				    [mips_arch_list_matching mips2 !r5900]
@@ -1353,9 +1353,9 @@ if { [istarget mips*-*-vxworks*] } {
 				    [mips_arch_list_matching mips32r2]
     run_list_test_arches "attr-gnu-4-7-64" "-64 -mfp64" \
 				    [mips_arch_list_matching mips3]
-    run_list_test_arches "attr-gnu-4-7" "-32 -msingle-float" \
+    run_list_test_arches "attr-gnu-4-7-msingle-float" "-32 -msingle-float" \
 				    [mips_arch_list_matching mips1]
-    run_list_test_arches "attr-gnu-4-7" "-32 -msoft-float" \
+    run_list_test_arches "attr-gnu-4-7-msoft-float" "-32 -msoft-float" \
 				    [mips_arch_list_matching mips1]
     run_list_test_arches "attr-gnu-4-7" "-32 -mfpxx" \
 				    [mips_arch_list_matching mips2 !r5900]


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks
  2014-09-10 20:48 [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks Matthew Fortune
@ 2014-09-13  8:19 ` Richard Sandiford
  2014-09-13  9:13   ` Matthew Fortune
  2014-09-13 22:48   ` Matthew Fortune
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Sandiford @ 2014-09-13  8:19 UTC (permalink / raw)
  To: Matthew Fortune; +Cc: binutils, Andrew Bennett

Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> This patch fixes a subtle mistake in the FP ABI consistency check
> logic.  The error reporting does not currently follow the
> intended precedence of the various FP ABIs. I.e. softfloat,
> singlefloat and then all the hardfloat variants.  When someone
> uses -msingle-float with a GNU attribute which is not 4,2 then
> the initial warning should be that it is not compatible with
> singlefloat. Likewise for softfloat and attribute 4,3.

I agree with checking soft/single/double before register size,
but the patch still does it after checking -mabi.  Is that important?
I've always thought of -mabi and -mgp/-mfp being a set so IMO it's more
natural to check soft/single/double first, then ABI, then register size.
E.g. if someone uses fpxx in a softfloat n32 then I think it's valid to
report either the softfloatness or the n32ness being the problem.

It'd be cleaner to have just one copy of the code at the head of the
function rather than duplicate it in each case statement.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks
  2014-09-13  8:19 ` Richard Sandiford
@ 2014-09-13  9:13   ` Matthew Fortune
  2014-09-13 22:48   ` Matthew Fortune
  1 sibling, 0 replies; 5+ messages in thread
From: Matthew Fortune @ 2014-09-13  9:13 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils, Andrew Bennett

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> > This patch fixes a subtle mistake in the FP ABI consistency check
> > logic.  The error reporting does not currently follow the
> > intended precedence of the various FP ABIs. I.e. softfloat,
> > singlefloat and then all the hardfloat variants.  When someone
> > uses -msingle-float with a GNU attribute which is not 4,2 then
> > the initial warning should be that it is not compatible with
> > singlefloat. Likewise for softfloat and attribute 4,3.
> 
> I agree with checking soft/single/double before register size,
> but the patch still does it after checking -mabi.  Is that important?
> I've always thought of -mabi and -mgp/-mfp being a set so IMO it's more
> natural to check soft/single/double first, then ABI, then register size.
> E.g. if someone uses fpxx in a softfloat n32 then I think it's valid to
> report either the softfloatness or the n32ness being the problem.
> 
> It'd be cleaner to have just one copy of the code at the head of the
> function rather than duplicate it in each case statement.

I think that sounds OK. I just had myself focussed on keeping the
O32-only ABI extensions restricted to O32 but ordering does not matter
for this particular piece of code.

Thanks,
Matthew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks
  2014-09-13  8:19 ` Richard Sandiford
  2014-09-13  9:13   ` Matthew Fortune
@ 2014-09-13 22:48   ` Matthew Fortune
  2014-09-14 22:36     ` Richard Sandiford
  1 sibling, 1 reply; 5+ messages in thread
From: Matthew Fortune @ 2014-09-13 22:48 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils, Andrew Bennett

> It'd be cleaner to have just one copy of the code at the head of the
> function rather than duplicate it in each case statement.

I've ended up with something that is still quite complex as I have
accounted for all of:

1) An unknown fpabi must be reported as such rather than being
   incompatible with or requiring any specific option.
2) fpabi 4 is unsupported regardless.
3) All softfloat checks had to be pulled up to the top to ensure
   that the single float check did not precede any of the softfloat
   checks. (So I moved all of single and soft to the top).
4) Ensure that only one warning comes out.

I also restated all the inconsistencies in terms of what option is
required to make the fpabi valid rather than sometimes saying what
is incompatible.

What do you think of this set of checks? I don't mind going over this
again and trying something else if you have any more suggestions or
concerns.

Thanks,
Matthew

gas/

	* tc-mips.c (fpabi_incompatible_with): Remove.
	(check_fpabi): Move softfloat and single float checks higher.
	Convert all warnings to state requirements rather than
	incompatibilities.

gas/testsuite/

	* gas/mips/attr-gnu-4-5-msingle-float.l: New file.
	* gas/mips/attr-gnu-4-5-msingle-float.s: Likewise.
	* gas/mips/attr-gnu-4-5-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-5-msoft-float.s: Likewise.
	* gas/mips/attr-gnu-4-1-mfp32.l: Adjust warning message.
	* gas/mips/attr-gnu-4-1-mfp64.l: Likewise.
	* gas/mips/attr-gnu-4-1-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-2-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-6-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-7-msoft-float.l: Likewise.
	* gas/mips/attr-gnu-4-1-msingle-float.l: Likewise.
	* gas/mips/attr-gnu-4-6-msingle-float.l: Likewise.
	* gas/mips/attr-gnu-4-7-msingle-float.l: Likewise.
	* gas/mips/attr-gnu-4-6-noodd.l: Likewise.
	* gas/mips/mips.exp: Update expected output for attr-gnu 4 tests.
---
 gas/config/tc-mips.c                               | 119 ++++++++++-----------
 gas/testsuite/gas/mips/attr-gnu-4-1-mfp32.l        |   2 +-
 gas/testsuite/gas/mips/attr-gnu-4-1-mfp64.l        |   2 +-
 .../gas/mips/attr-gnu-4-1-msingle-float.l          |   2 +-
 gas/testsuite/gas/mips/attr-gnu-4-1-msoft-float.l  |   2 +-
 gas/testsuite/gas/mips/attr-gnu-4-2-msoft-float.l  |   2 +-
 .../gas/mips/attr-gnu-4-5-msingle-float.l          |   2 +
 .../gas/mips/attr-gnu-4-5-msingle-float.s          |   1 +
 gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l  |   2 +
 gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s  |   1 +
 .../gas/mips/attr-gnu-4-6-msingle-float.l          |   2 +-
 gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l  |   2 +-
 gas/testsuite/gas/mips/attr-gnu-4-6-noodd.l        |   2 +-
 .../gas/mips/attr-gnu-4-7-msingle-float.l          |   2 +-
 gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l  |   2 +-
 gas/testsuite/gas/mips/mips.exp                    |  12 +--
 16 files changed, 77 insertions(+), 80 deletions(-)
 create mode 100644 gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l
 create mode 100644 gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s
 create mode 100644 gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l
 create mode 100644 gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 59d8635..759ee4b 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -3655,13 +3655,6 @@ md_begin (void)
 }
 
 static inline void
-fpabi_incompatible_with (int fpabi, const char *what)
-{
-  as_warn (_(".gnu_attribute %d,%d is incompatible with `%s'"),
-	   Tag_GNU_MIPS_ABI_FP, fpabi, what);
-}
-
-static inline void
 fpabi_requires (int fpabi, const char *what)
 {
   as_warn (_(".gnu_attribute %d,%d requires `%s'"),
@@ -3672,68 +3665,66 @@ fpabi_requires (int fpabi, const char *what)
 static void
 check_fpabi (int fpabi)
 {
-  bfd_boolean needs_check = FALSE;
-  switch (fpabi)
-    {
-    case Val_GNU_MIPS_ABI_FP_DOUBLE:
-      if (file_mips_opts.gp == 64 && file_mips_opts.fp == 32)
-	fpabi_incompatible_with (fpabi, "gp=64 fp=32");
-      else if (file_mips_opts.gp == 32 && file_mips_opts.fp == 64)
-	fpabi_incompatible_with (fpabi, "gp=32 fp=64");
-      else
-	needs_check = TRUE;
-      break;
-
-    case Val_GNU_MIPS_ABI_FP_XX:
-      if (mips_abi != O32_ABI)
-	fpabi_requires (fpabi, "-mabi=32");
-      else if (file_mips_opts.fp != 0)
-	fpabi_requires (fpabi, "fp=xx");
-      else
-	needs_check = TRUE;
-      break;
-
-    case Val_GNU_MIPS_ABI_FP_64A:
-    case Val_GNU_MIPS_ABI_FP_64:
-      if (mips_abi != O32_ABI)
-	fpabi_requires (fpabi, "-mabi=32");
-      else if (file_mips_opts.fp != 64)
-	fpabi_requires (fpabi, "fp=64");
-      else if (fpabi == Val_GNU_MIPS_ABI_FP_64 && !file_mips_opts.oddspreg)
-	fpabi_incompatible_with (fpabi, "nooddspreg");
-      else if (fpabi == Val_GNU_MIPS_ABI_FP_64A && file_mips_opts.oddspreg)
-	fpabi_requires (fpabi, "nooddspreg");
-      else
-	needs_check = TRUE;
-      break;
+  if (fpabi == Val_GNU_MIPS_ABI_FP_SOFT && !file_mips_opts.soft_float)
+    fpabi_requires (fpabi, "softfloat");
+  else if ((fpabi == Val_GNU_MIPS_ABI_FP_DOUBLE
+	    || fpabi == Val_GNU_MIPS_ABI_FP_SINGLE
+	    || fpabi == Val_GNU_MIPS_ABI_FP_XX
+	    || fpabi == Val_GNU_MIPS_ABI_FP_64
+	    || fpabi == Val_GNU_MIPS_ABI_FP_64A)
+	   && file_mips_opts.soft_float)
+    fpabi_requires (fpabi, "hardfloat");
+  else if (fpabi == Val_GNU_MIPS_ABI_FP_SINGLE && !file_mips_opts.single_float)
+    fpabi_requires (fpabi, "singlefloat");
+  else if ((fpabi == Val_GNU_MIPS_ABI_FP_DOUBLE
+	    || fpabi == Val_GNU_MIPS_ABI_FP_XX
+	    || fpabi == Val_GNU_MIPS_ABI_FP_64
+	    || fpabi == Val_GNU_MIPS_ABI_FP_64A)
+	   && file_mips_opts.single_float)
+    fpabi_requires (fpabi, "doublefloat");
+  else
+    switch (fpabi)
+      {
+      case Val_GNU_MIPS_ABI_FP_DOUBLE:
+	if (file_mips_opts.gp == 64 && file_mips_opts.fp == 32)
+	  fpabi_requires (fpabi, "fp=64");
+	else if (file_mips_opts.gp == 32 && file_mips_opts.fp == 64)
+	  fpabi_requires (fpabi, "fp=32");
+	break;
 
-    case Val_GNU_MIPS_ABI_FP_SINGLE:
-      if (file_mips_opts.soft_float)
-	fpabi_incompatible_with (fpabi, "softfloat");
-      else if (!file_mips_opts.single_float)
-	fpabi_requires (fpabi, "singlefloat");
-      break;
+      case Val_GNU_MIPS_ABI_FP_XX:
+	if (mips_abi != O32_ABI)
+	  fpabi_requires (fpabi, "-mabi=32");
+	else if (file_mips_opts.fp != 0)
+	  fpabi_requires (fpabi, "fp=xx");
+	break;
 
-    case Val_GNU_MIPS_ABI_FP_SOFT:
-      if (!file_mips_opts.soft_float)
-	fpabi_requires (fpabi, "softfloat");
-      break;
+      case Val_GNU_MIPS_ABI_FP_64A:
+      case Val_GNU_MIPS_ABI_FP_64:
+	if (mips_abi != O32_ABI)
+	  fpabi_requires (fpabi, "-mabi=32");
+	else if (file_mips_opts.fp != 64)
+	  fpabi_requires (fpabi, "fp=64");
+	else if (fpabi == Val_GNU_MIPS_ABI_FP_64 && !file_mips_opts.oddspreg)
+	  fpabi_requires (fpabi, "oddspreg");
+	else if (fpabi == Val_GNU_MIPS_ABI_FP_64A && file_mips_opts.oddspreg)
+	  fpabi_requires (fpabi, "nooddspreg");
+	break;
 
-    case Val_GNU_MIPS_ABI_FP_OLD_64:
-      as_warn (_(".gnu_attribute %d,%d is no longer supported"),
-	       Tag_GNU_MIPS_ABI_FP, fpabi);
-      break;
+      case Val_GNU_MIPS_ABI_FP_SOFT:
+      case Val_GNU_MIPS_ABI_FP_SINGLE:
+	break;
 
-    default:
-      as_warn (_(".gnu_attribute %d,%d is not a recognized"
-	         " floating-point ABI"), Tag_GNU_MIPS_ABI_FP, fpabi);
-      break;
-    }
+      case Val_GNU_MIPS_ABI_FP_OLD_64:
+	as_warn (_(".gnu_attribute %d,%d is no longer supported"),
+		 Tag_GNU_MIPS_ABI_FP, fpabi);
+	break;
 
-  if (needs_check && file_mips_opts.soft_float)
-    fpabi_incompatible_with (fpabi, "softfloat");
-  else if (needs_check && file_mips_opts.single_float)
-    fpabi_incompatible_with (fpabi, "singlefloat");
+      default:
+	as_warn (_(".gnu_attribute %d,%d is not a recognized"
+		   " floating-point ABI"), Tag_GNU_MIPS_ABI_FP, fpabi);
+	break;
+      }
 }
 
 /* Perform consistency checks on the current options.  */
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-1-mfp32.l b/gas/testsuite/gas/mips/attr-gnu-4-1-mfp32.l
index 96db3ce..2208650 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-1-mfp32.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-1-mfp32.l
@@ -1,3 +1,3 @@
 .*: Assembler messages:
 .*: Warning: `fp=32' used with a 64-bit ABI
-.*: Warning: .gnu_attribute 4,1 is incompatible with `gp=64 fp=32'
+.*: Warning: .gnu_attribute 4,1 requires `fp=64'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-1-mfp64.l b/gas/testsuite/gas/mips/attr-gnu-4-1-mfp64.l
index 78b5fc4..67f3876 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-1-mfp64.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-1-mfp64.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,1 is incompatible with `gp=32 fp=64'
+.*: Warning: .gnu_attribute 4,1 requires `fp=32'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-1-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-1-msingle-float.l
index c37c520..012a0e9 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-1-msingle-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-1-msingle-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,1 is incompatible with `singlefloat'
+.*: Warning: .gnu_attribute 4,1 requires `doublefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-1-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-1-msoft-float.l
index 819abfa..21b289f 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-1-msoft-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-1-msoft-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,1 is incompatible with `softfloat'
+.*: Warning: .gnu_attribute 4,1 requires `hardfloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-2-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-2-msoft-float.l
index 9d421bd..5357150 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-2-msoft-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-2-msoft-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,2 is incompatible with `softfloat'
+.*: Warning: .gnu_attribute 4,2 requires `hardfloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l
new file mode 100644
index 0000000..ccb591f
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*: Warning: .gnu_attribute 4,5 requires `doublefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s
new file mode 100644
index 0000000..b21ec3b
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msingle-float.s
@@ -0,0 +1 @@
+.gnu_attribute 4,5
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l
new file mode 100644
index 0000000..54ac42d
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*: Warning: .gnu_attribute 4,5 requires `hardfloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s
new file mode 100644
index 0000000..b21ec3b
--- /dev/null
+++ b/gas/testsuite/gas/mips/attr-gnu-4-5-msoft-float.s
@@ -0,0 +1 @@
+.gnu_attribute 4,5
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l
index 999b5e2..78afe57 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-6-msingle-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 requires `doublefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l
index 999b5e2..a9b9cb4 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-6-msoft-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 requires `hardfloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-6-noodd.l b/gas/testsuite/gas/mips/attr-gnu-4-6-noodd.l
index e885317..03dde84 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-6-noodd.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-6-noodd.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `nooddspreg'
+.*: Warning: .gnu_attribute 4,6 requires `oddspreg'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l b/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l
index 999b5e2..78afe57 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-7-msingle-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 requires `doublefloat'
diff --git a/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l b/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l
index 999b5e2..a9b9cb4 100644
--- a/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l
+++ b/gas/testsuite/gas/mips/attr-gnu-4-7-msoft-float.l
@@ -1,2 +1,2 @@
 .*: Assembler messages:
-.*: Warning: .gnu_attribute 4,6 is incompatible with `fp=32'
+.*: Warning: .gnu_attribute 4,6 requires `hardfloat'
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index 198d60e..4cf6e7d 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -1327,9 +1327,9 @@ if { [istarget mips*-*-vxworks*] } {
 				    [mips_arch_list_matching mips32r2]
     run_list_test_arches "attr-gnu-4-5-64" "-64 -mfp64" \
 				    [mips_arch_list_matching mips3]
-    run_list_test_arches "attr-gnu-4-5" "-32 -msingle-float" \
+    run_list_test_arches "attr-gnu-4-5-msingle-float" "-32 -msingle-float" \
 				    [mips_arch_list_matching mips1]
-    run_list_test_arches "attr-gnu-4-5" "-32 -msoft-float" \
+    run_list_test_arches "attr-gnu-4-5-msoft-float" "-32 -msoft-float" \
 				    [mips_arch_list_matching mips1]
     run_dump_test_arches "attr-gnu-4-5" \
 				    [mips_arch_list_matching mips2 !r5900]
@@ -1340,9 +1340,9 @@ if { [istarget mips*-*-vxworks*] } {
 				    [mips_arch_list_matching mips32r2]
     run_list_test_arches "attr-gnu-4-6-64" "-64 -mfp64" \
 				    [mips_arch_list_matching mips3]
-    run_list_test_arches "attr-gnu-4-6" "-32 -msingle-float" \
+    run_list_test_arches "attr-gnu-4-6-msingle-float" "-32 -msingle-float" \
 				    [mips_arch_list_matching mips1]
-    run_list_test_arches "attr-gnu-4-6" "-32 -msoft-float" \
+    run_list_test_arches "attr-gnu-4-6-msoft-float" "-32 -msoft-float" \
 				    [mips_arch_list_matching mips1]
     run_list_test_arches "attr-gnu-4-6" "-32 -mfpxx" \
 				    [mips_arch_list_matching mips2 !r5900]
@@ -1355,9 +1355,9 @@ if { [istarget mips*-*-vxworks*] } {
 				    [mips_arch_list_matching mips32r2]
     run_list_test_arches "attr-gnu-4-7-64" "-64 -mfp64" \
 				    [mips_arch_list_matching mips3]
-    run_list_test_arches "attr-gnu-4-7" "-32 -msingle-float" \
+    run_list_test_arches "attr-gnu-4-7-msingle-float" "-32 -msingle-float" \
 				    [mips_arch_list_matching mips1]
-    run_list_test_arches "attr-gnu-4-7" "-32 -msoft-float" \
+    run_list_test_arches "attr-gnu-4-7-msoft-float" "-32 -msoft-float" \
 				    [mips_arch_list_matching mips1]
     run_list_test_arches "attr-gnu-4-7" "-32 -mfpxx" \
 				    [mips_arch_list_matching mips2 !r5900]
-- 
1.9.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks
  2014-09-13 22:48   ` Matthew Fortune
@ 2014-09-14 22:36     ` Richard Sandiford
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Sandiford @ 2014-09-14 22:36 UTC (permalink / raw)
  To: Matthew Fortune; +Cc: binutils, Andrew Bennett

Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
>> It'd be cleaner to have just one copy of the code at the head of the
>> function rather than duplicate it in each case statement.
>
> I've ended up with something that is still quite complex as I have
> accounted for all of:
>
> 1) An unknown fpabi must be reported as such rather than being
>    incompatible with or requiring any specific option.
> 2) fpabi 4 is unsupported regardless.
> 3) All softfloat checks had to be pulled up to the top to ensure
>    that the single float check did not precede any of the softfloat
>    checks. (So I moved all of single and soft to the top).
> 4) Ensure that only one warning comes out.
>
> I also restated all the inconsistencies in terms of what option is
> required to make the fpabi valid rather than sometimes saying what
> is incompatible.
>
> What do you think of this set of checks? I don't mind going over this
> again and trying something else if you have any more suggestions or
> concerns.

Yeah, that's not pretty either. :-(  Code getting this messy seems
like a good indication that we're not using the right representation.
It really shouldn't be this hard or need this much cut-&-paste...

Ah well.  Let's go with your original patch.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-09-14 22:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10 20:48 [PATCH, MIPS] Ensure softfloat and single float take precendence in consistency checks Matthew Fortune
2014-09-13  8:19 ` Richard Sandiford
2014-09-13  9:13   ` Matthew Fortune
2014-09-13 22:48   ` Matthew Fortune
2014-09-14 22:36     ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).