[PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[Jim Wilson]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

The mipsisa32r2-sdemtk-elf target supports a -mno-float option, which
disallows any FP operations.  It uses the -msoft-float ABI, and then
links with a library without the soft-float routines.  Since it doesn't
allow any FP operations, it is ABI compatible with all other ABIs with
respect to FP support.

The MIPS port uses .gnu.attribute 4,X to describe the FP ABI being used.
We need to emit 0 for the -mno-float option, to indicate that we can
link with any FP ABI.

I expanded the .gnu.attribute support and added some comments to make it
clearer what was going on.    Then added the SUBTARGET_AS_GNU_ATTRIBUTE
macro so that the sdemtk port can handle -mno-float.

This was tested with a mipsisa32r2-sde-elf build and testsuite run for C
and C++.  There were no regressions.  This was tested by hand on small
testcases to verify that I got the correct correct in all cases for both
the sde and sdemtk toolchains.

OK?

Jim


[-- Attachment #2: sdemtk-gnu-attr.patch --]
[-- Type: text/x-patch, Size: 1848 bytes --]

2010-04-23  James E. Wilson  <wilson@codesourcery.com>

	* config/mips/sdemtk.h: (SUBTARGET_AS_GNU_ATTRIBUTE): New.
	* config/mips/mips.c (mips_file_start): Call it.

Index: sdemtk.h
===================================================================
--- sdemtk.h	(revision 158495)
+++ sdemtk.h	(working copy)
@@ -113,3 +113,9 @@ extern void mips_sync_icache (void *beg,
 /* ...nor does the call sequence preserve $31.  */
 #undef MIPS_SAVE_REG_FOR_PROFILING_P
 #define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) ((REGNO) == RETURN_ADDR_REGNUM)
+
+/* TARGET_NO_FLOAT code has no float code at all, and hence is compatible with
+   all other float ABIs, so we set the attribute value to 0.  */
+#undef SUBTARGET_AS_GNU_ATTRIBUTE
+#define SUBTARGET_AS_GNU_ATTRIBUTE(ATTR)				\
+  do { if (TARGET_NO_FLOAT) ATTR = 0; } while (0)
Index: mips.c
===================================================================
--- mips.c	(revision 158495)
+++ mips.c	(working copy)
@@ -8185,10 +8185,28 @@ mips_file_start (void)
 		 "\t.previous\n", TARGET_LONG64 ? 64 : 32);
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-      fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n",
-	       (TARGET_HARD_FLOAT_ABI
-		? (TARGET_DOUBLE_FLOAT
-		   ? ((!TARGET_64BIT && TARGET_FLOAT64) ? 4 : 1) : 2) : 3));
+      {
+	int attr;
+
+	/* Soft-float code, -msoft-float.  */
+	if (!TARGET_HARD_FLOAT_ABI)
+	  attr = 3;
+	/* Single-float code, -msingle-float.  */
+	else if (!TARGET_DOUBLE_FLOAT)
+	  attr = 2;
+	/* 64-bit FP registers on a 32-bit target, -mips32r2 -mfp64.  */
+	else if (!TARGET_64BIT && TARGET_FLOAT64)
+	  attr = 4;
+	/* Regular FP code, FP regs same size as GP regs, -mdouble-float.  */
+	else
+	  attr = 1;
+
+#ifdef SUBTARGET_AS_GNU_ATTRIBUTE
+	SUBTARGET_AS_GNU_ATTRIBUTE(attr);
+#endif
+
+	fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n", attr);
+      }
 #endif
     }
 

Re: [PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[Richard Sandiford]

This may be OK, but I want to make sure I understand.

I assume the point of this patch is that you can't currently link
-mno-float objects with -mhard-float objects.  If so...

Jim Wilson <wilson@codesourcery.com> writes:
> The mipsisa32r2-sdemtk-elf target supports a -mno-float option, which
> disallows any FP operations.  It uses the -msoft-float ABI, and then
> links with a library without the soft-float routines.  Since it doesn't
> allow any FP operations, it is ABI compatible with all other ABIs with
> respect to FP support.

...why is it relevant that -mno-float links to a library without the
soft-float routines (or indeed any float routines)?  Presumably you
wouldn't be linking with -mno-float if you were combining -mhard-float
and -mno-float objects.

The problem is that I can compile:

  float f (float x, float y) { return x + y; }

using -mno-float without error.  It generates a call to __addsf3.
Now this is admittedly user error, but it isn't flagged by the compiler.
We rely on the linker (and the absence of soft-float routines) to catch
the mistake.

Or to put it another way: -mno-float only differs from -msoft-float in the
way that its libraries are constructed.  There is no difference in the
compiler itself beyond the set of preprocessor macros that are defined.
I'm uncomfortable with the idea of .gnu_attribute being used to model
things that are related to preprocessor macros rather than code generation.

That's not an outright rejection.  I'd just like to be sure of the
rationale.

Richard

Re: [PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[Jim Wilson]

On Sat, 2010-05-01 at 09:18 +0100, Richard Sandiford wrote:
> ...why is it relevant that -mno-float links to a library without the
> soft-float routines (or indeed any float routines)?

I was just trying to explain what -mno-float does.  I am not sure who
all is familiar with it, as only the sdemtk target supports it, and this
target was apparently not actively maintained in the FSF tree.

> Or to put it another way: -mno-float only differs from -msoft-float in the
> way that its libraries are constructed.  There is no difference in the
> compiler itself beyond the set of preprocessor macros that are defined.
> I'm uncomfortable with the idea of .gnu_attribute being used to model
> things that are related to preprocessor macros rather than code generation.

I don't know the original justification for this work.  It was done at
MTI, and most of the original people aren't available for questions
anymore.  I can see a purpose here though.

Suppose you are a 3rd party library vendor, and you want to build and
distribute a library compatible with an MTI and/or CodeSourcery
toolchain.  Your library does not use any FP code.  And yet, because of
the FP ABI encoded into .gnu_attribute, you must build and distribute 4
different versions of your library compiled for the 4 different FP ABIs.
The -mno-float option solves this problem.  It is a compile time
assertion that there is no FP code, and hence the result is compatible
with all 4 FP ABIs.  If you compile with -mno-float, you now need only
one version of the library for the different FP ABIs.

It is true, the current implementation of -mno-float only differs in
library construction and preprocessor macros.  It is possible that
future implementations could do more to verify the compile-time
assertion.  Meanwhile, however, the main purpose of the -mno-float
option is currently not being served, because an important piece is
missing, the gnu_attribute setting.  I believe the .gnu_attribute patch
should be added in, irrespective of whether the -mno-float
implementation is improved.  I think improving the -mno-float
implementation to do more compile-time checking for FP code should be a
separate issue.

Jim

Re: [PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[Richard Sandiford]

Jim Wilson <wilson@codesourcery.com> writes:
> Suppose you are a 3rd party library vendor, and you want to build and
> distribute a library compatible with an MTI and/or CodeSourcery
> toolchain.  Your library does not use any FP code.  And yet, because of
> the FP ABI encoded into .gnu_attribute, you must build and distribute 4
> different versions of your library compiled for the 4 different FP ABIs.
> The -mno-float option solves this problem.  It is a compile time
> assertion that there is no FP code, and hence the result is compatible
> with all 4 FP ABIs.  If you compile with -mno-float, you now need only
> one version of the library for the different FP ABIs.

Right, I appreciate that.  It was more...

> It is true, the current implementation of -mno-float only differs in
> library construction and preprocessor macros.  It is possible that
> future implementations could do more to verify the compile-time
> assertion.  Meanwhile, however, the main purpose of the -mno-float
> option is currently not being served, because an important piece is
> missing, the gnu_attribute setting.  I believe the .gnu_attribute patch
> should be added in, irrespective of whether the -mno-float
> implementation is improved.  I think improving the -mno-float
> implementation to do more compile-time checking for FP code should be a
> separate issue.

...this that I was worried about.  We wouldn't want the same behaviour
for -msoft-float -DSOME_MACRO, even though that's actually entirely
equivalent to what -mno-float does from a compiler perspective.

I think this is one of those situations where neither of us is going to
convince the other.  Since I've noted my reservations, and since it's an
SDE-specific patch, I'll back down.

However, I don't like the idea of hiding the option behind even more
hooks.  I'd prefer to expose TARGET_NO_FLOAT to the generic code instead.
Any objections to the following alternative?  Tested on mipsisa64-elf
and by inspection on mipsisa64-sde-elf.

I've kept the option specific to SDE targets for now.  I'd be happy
to open it up to other targets in future if the lack of FP operations
is ever properly enforced.

Richard


2010-05-08  Richard Sandiford  <rdsandiford@googlemail.com>
	    Jim Wilson  <wilson@codesourcery.com>

gcc/
	* config.gcc (mips*-sde-elf*): Don't use sdemtk.opt.
	* config/mips/mips.h (TARGET_CPU_CPP_BUILTINS): Define __mips_no_float
	for TARGET_NO_FLOAT.
	* config/mips/mips.c (mips_file_start): Expand conditional expression
	into "if" statements.  Use .gnu_attribute 4,0 for TARGET_NO_FLOAT.
	(mips_override_options): Move -mno-float override -msoft-float and
	-mhard-float.
	* config/mips/mips.opt (mno-float): Move from sdemtk.opt, but add
	Condition(TARGET_SUPPORTS_NO_FLOAT).
	* config/mips/sdemtk.h (TARGET_OS_CPP_BUILTINS): Don't set
	__mips_no_float here.
	(SUBTARGET_OVERRIDE_OPTIONS): Delete.
	(TARGET_SUPPORTS_NO_FLOAT): Define.
	* config/mips/sdemtk.opt: Delete.

Index: gcc/config.gcc
===================================================================
--- gcc/config.gcc	2010-05-08 08:46:49.000000000 +0100
+++ gcc/config.gcc	2010-05-08 08:46:56.000000000 +0100
@@ -1764,7 +1764,6 @@ mips*-sde-elf*)
 	    # MIPS toolkit libraries.
 	    tm_file="$tm_file mips/sdemtk.h"
 	    tmake_file="$tmake_file mips/t-sdemtk"
-	    extra_options="$extra_options mips/sdemtk.opt"
 	    case ${enable_threads} in
 	      "" | yes | mipssde)
 		thread_file='mipssde'
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	2010-05-08 08:48:57.000000000 +0100
+++ gcc/config/mips/mips.h	2010-05-08 08:49:22.000000000 +0100
@@ -537,7 +537,9 @@ #define TARGET_CPU_CPP_BUILTINS()					\
 									\
       /* These defines reflect the ABI in use, not whether the  	\
 	 FPU is directly accessible.  */				\
-      if (TARGET_HARD_FLOAT_ABI)					\
+      if (TARGET_NO_FLOAT)						\
+	builtin_define ("__mips_no_float");				\
+      else if (TARGET_HARD_FLOAT_ABI)					\
 	builtin_define ("__mips_hard_float");				\
       else								\
 	builtin_define ("__mips_soft_float");				\
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2010-05-08 08:51:55.000000000 +0100
+++ gcc/config/mips/mips.c	2010-05-08 09:01:56.000000000 +0100
@@ -8185,10 +8185,27 @@ mips_file_start (void)
 		 "\t.previous\n", TARGET_LONG64 ? 64 : 32);
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-      fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n",
-	       (TARGET_HARD_FLOAT_ABI
-		? (TARGET_DOUBLE_FLOAT
-		   ? ((!TARGET_64BIT && TARGET_FLOAT64) ? 4 : 1) : 2) : 3));
+      {
+	int attr;
+
+	/* No floating-point operations, -mno-float.  */
+	if (TARGET_NO_FLOAT)
+	  attr = 0;
+	/* Soft-float code, -msoft-float.  */
+	else if (!TARGET_HARD_FLOAT_ABI)
+	  attr = 3;
+	/* Single-float code, -msingle-float.  */
+	else if (!TARGET_DOUBLE_FLOAT)
+	  attr = 2;
+	/* 64-bit FP registers on a 32-bit target, -mips32r2 -mfp64.  */
+	else if (!TARGET_64BIT && TARGET_FLOAT64)
+	  attr = 4;
+	/* Regular FP code, FP regs same size as GP regs, -mdouble-float.  */
+	else
+	  attr = 1;
+
+	fprintf (asm_out_file, "\t.gnu_attribute 4, %d\n", attr);
+      }
 #endif
     }
 
@@ -15389,6 +15406,13 @@ mips_override_options (void)
   SUBTARGET_OVERRIDE_OPTIONS;
 #endif
 
+  /* -mno-float overrides -mhard-float and -msoft-float.  */
+  if (TARGET_NO_FLOAT)
+    {
+      target_flags |= MASK_SOFT_FLOAT_ABI;
+      target_flags_explicit |= MASK_SOFT_FLOAT_ABI;
+    }
+
   /* Set the small data limit.  */
   mips_small_data_threshold = (g_switch_set
 			       ? g_switch_value
Index: gcc/config/mips/mips.opt
===================================================================
--- gcc/config/mips/mips.opt	2010-05-08 08:57:22.000000000 +0100
+++ gcc/config/mips/mips.opt	2010-05-08 08:57:50.000000000 +0100
@@ -224,6 +224,10 @@ mmt
 Target Report Var(TARGET_MT)
 Allow the use of MT instructions
 
+mno-float
+Target Report RejectNegative Var(TARGET_NO_FLOAT) Condition(TARGET_SUPPORTS_NO_FLOAT)
+Prevent the use of all floating-point operations
+
 mno-flush-func
 Target RejectNegative
 Do not use a cache-flushing function before calling stack trampolines
Index: gcc/config/mips/sdemtk.h
===================================================================
--- gcc/config/mips/sdemtk.h	2010-05-08 08:48:45.000000000 +0100
+++ gcc/config/mips/sdemtk.h	2010-05-08 08:58:41.000000000 +0100
@@ -35,10 +35,7 @@ #define TARGET_OS_CPP_BUILTINS()			\
 	builtin_define ("__mipsfp64");			\
 							\
       if (TARGET_NO_FLOAT) 				\
-	{						\
-	  builtin_define ("__NO_FLOAT");		\
-	  builtin_define ("__mips_no_float");		\
-	}						\
+	builtin_define ("__NO_FLOAT");			\
       else if (TARGET_SOFT_FLOAT_ABI)			\
 	builtin_define ("__SOFT_FLOAT");		\
       else if (TARGET_SINGLE_FLOAT)			\
@@ -57,18 +54,6 @@ #define TARGET_OS_CPP_BUILTINS()			\
     }							\
   while (0)
 
-#undef SUBTARGET_OVERRIDE_OPTIONS
-#define SUBTARGET_OVERRIDE_OPTIONS			\
-  do							\
-    {							\
-      if (TARGET_NO_FLOAT)				\
-	{						\
-	  target_flags |= MASK_SOFT_FLOAT_ABI;		\
-	  target_flags_explicit |= MASK_SOFT_FLOAT_ABI;	\
-	}						\
-    }							\
-  while (0)
-
 /* For __clear_cache in libgcc2.c.  */
 #ifdef IN_LIBGCC2
 extern void mips_sync_icache (void *beg, unsigned long len);
@@ -113,3 +98,6 @@ #define FUNCTION_PROFILER(FILE, LABELNO)
 /* ...nor does the call sequence preserve $31.  */
 #undef MIPS_SAVE_REG_FOR_PROFILING_P
 #define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) ((REGNO) == RETURN_ADDR_REGNUM)
+
+/* Compile in support for the -mno-float option.  */
+#define TARGET_SUPPORTS_NO_FLOAT 1
Index: gcc/config/mips/sdemtk.opt
===================================================================
--- gcc/config/mips/sdemtk.opt	2010-05-08 08:58:54.000000000 +0100
+++ /dev/null	2010-05-08 09:05:06.395398768 +0100
@@ -1,23 +0,0 @@
-; Options for the MIPS SDE configuration.
-;
-; Copyright (C) 2007 Free Software Foundation, Inc.
-;
-; This file is part of GCC.
-;
-; GCC is free software; you can redistribute it and/or modify it under
-; the terms of the GNU General Public License as published by the Free
-; Software Foundation; either version 3, or (at your option) any later
-; version.
-;
-; GCC is distributed in the hope that it will be useful, but WITHOUT
-; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
-; or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
-; License for more details.
-;
-; You should have received a copy of the GNU General Public License
-; along with GCC; see the file COPYING3.  If not see
-; <http://www.gnu.org/licenses/>.
-
-mno-float
-Target Report RejectNegative Var(TARGET_NO_FLOAT)
-Prevent the use of all floating-point operations

Re: [PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[Jim Wilson]

On Sat, 2010-05-08 at 17:55 +0100, Richard Sandiford wrote:
> However, I don't like the idea of hiding the option behind even more
> hooks.  I'd prefer to expose TARGET_NO_FLOAT to the generic code instead.
> Any objections to the following alternative?  Tested on mipsisa64-elf
> and by inspection on mipsisa64-sde-elf.

No objections.  This looks fine to me.

And I'm the one that needs to apologize now.  I took some planned time
off about the time you returned, and then I had to take some unplanned
time off, and I haven't been managing my backlog of FSF work very well
since then.

Jim

Re: [PATCH, MIPS] .gnu.attribute support for sdemtk -mno-float

[Richard Sandiford]

Jim Wilson <wilson@codesourcery.com> writes:
> On Sat, 2010-05-08 at 17:55 +0100, Richard Sandiford wrote:
>> However, I don't like the idea of hiding the option behind even more
>> hooks.  I'd prefer to expose TARGET_NO_FLOAT to the generic code instead.
>> Any objections to the following alternative?  Tested on mipsisa64-elf
>> and by inspection on mipsisa64-sde-elf.
>
> No objections.  This looks fine to me.

Thanks, applied.

Richard