[RFC PATCH V2] MIPS: fix build with binutils 2.24.51+

[Manuel Lauss]

With binutils snapshots since 29.07.2014 I get the following build failure:
{standard input}: Warning: .gnu_attribute 4,3 requires `softfloat'
LD arch/mips/alchemy/common/built-in.o
mipsel-softfloat-linux-gnu-ld: Warning: arch/mips/alchemy/common/built-in.o
uses -msoft-float (set by arch/mips/alchemy/common/prom.o),
arch/mips/alchemy/common/sleeper.o uses -mhard-float

Extend cflags with a soft-float directive for the assembler, and add
hardfloat directives to assembler files dealing with FPU
registers to compensate.

Signed-off-by: Manuel Lauss <***@gmail.com>
---
V2: cover more assembler files.

This was introduced in binutils commit 351cdf24d223290b15fa991e5052ec9e9bd1e284
("[MIPS] Implement O32 FPXX, FP64 and FP64A ABI extensions"), and it seems is only
a problem with my gcc 4.9 or a very recent snapshot of gcc-4.10.
With gcc-4.8, the source builds, with 4.9 it aborts after the above error,
with 4.10 (which apparently passes -msoft-float along to the assembler automatically)
the .hardfloat directives are required.

arch/mips/Makefile | 2 +-
arch/mips/kernel/r2300_switch.S | 1 +
arch/mips/kernel/r4k_fpu.S | 1 +
arch/mips/kernel/r4k_switch.S | 1 +
arch/mips/kernel/r6000_fpu.S | 1 +
5 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 9336509..cffbd49 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -88,7 +88,7 @@ all-$(CONFIG_SYS_SUPPORTS_ZBOOT)+= vmlinuz
# crossformat linking we rely on the elf2ecoff tool for format conversion.
#
cflags-y += -G 0 -mno-abicalls -fno-pic -pipe
-cflags-y += -msoft-float
+cflags-y += -msoft-float -Wa,-msoft-float
LDFLAGS_vmlinux += -G 0 -static -n -nostdlib
KBUILD_AFLAGS_MODULE += -mlong-calls
KBUILD_CFLAGS_MODULE += -mlong-calls
diff --git a/arch/mips/kernel/r2300_switch.S b/arch/mips/kernel/r2300_switch.S
index 20b7b04..f33bf8b 100644
--- a/arch/mips/kernel/r2300_switch.S
+++ b/arch/mips/kernel/r2300_switch.S
@@ -22,6 +22,7 @@
#include <asm/asmmacro.h>

.set mips1
+ .set hardfloat
.align 5

/*
diff --git a/arch/mips/kernel/r4k_fpu.S b/arch/mips/kernel/r4k_fpu.S
index 8352523..722962c 100644
--- a/arch/mips/kernel/r4k_fpu.S
+++ b/arch/mips/kernel/r4k_fpu.S
@@ -21,6 +21,7 @@

.macro EX insn, reg, src
.set push
+ .set hardfloat
.set nomacro
.ex\@: \insn \reg, \src
.set pop
diff --git a/arch/mips/kernel/r4k_switch.S b/arch/mips/kernel/r4k_switch.S
index 4c4ec18..5313b6f 100644
--- a/arch/mips/kernel/r4k_switch.S
+++ b/arch/mips/kernel/r4k_switch.S
@@ -34,6 +34,7 @@
* struct thread_info *next_ti, s32 fp_save)
*/
.align 5
+ .set hardfloat
LEAF(resume)
mfc0 t1, CP0_STATUS
LONG_S t1, THREAD_STATUS(a0)
diff --git a/arch/mips/kernel/r6000_fpu.S b/arch/mips/kernel/r6000_fpu.S
index da0fbe4..c13e3ff 100644
--- a/arch/mips/kernel/r6000_fpu.S
+++ b/arch/mips/kernel/r6000_fpu.S
@@ -17,6 +17,7 @@
#include <asm/regdef.h>

.set noreorder
+ .set hardfloat
.set mips2
/* Save floating point context */
LEAF(_save_fp_context)

[Ralf Baechle]

Post by Manuel Lauss
{standard input}: Warning: .gnu_attribute 4,3 requires `softfloat'
LD arch/mips/alchemy/common/built-in.o
mipsel-softfloat-linux-gnu-ld: Warning: arch/mips/alchemy/common/built-in.o
uses -msoft-float (set by arch/mips/alchemy/common/prom.o),
arch/mips/alchemy/common/sleeper.o uses -mhard-float
Extend cflags with a soft-float directive for the assembler, and add
hardfloat directives to assembler files dealing with FPU
registers to compensate.

I had a discussion about this with Maciej. He suggested that this
behavious of binutils should be taken a look at but also that we rather
should remove the -msoft-float option from the kernel and I support his
view.

I did add -msoft-float in 6218cf4410cfce7bc7e89834e73525b124625d4c
[[MIPS] Always pass -msoft-float.] in 2006 because back then there was a
wave of bug reports from people attempting to use hard fp in the kernel
which of course did result in FPR corruption. Adding -msoft-float made
sure that floating point operations would result in a link error because
the kernel does not supply a soft-fp library.

But maybe there are other methods to achieve the same - such as

#define float diediedie
#define double goboom

Ralf

[Maciej W. Rozycki]

Post by Ralf Baechle

Post by Manuel Lauss
{standard input}: Warning: .gnu_attribute 4,3 requires `softfloat'
LD arch/mips/alchemy/common/built-in.o
mipsel-softfloat-linux-gnu-ld: Warning: arch/mips/alchemy/common/built-in.o
uses -msoft-float (set by arch/mips/alchemy/common/prom.o),
arch/mips/alchemy/common/sleeper.o uses -mhard-float
Extend cflags with a soft-float directive for the assembler, and add
hardfloat directives to assembler files dealing with FPU
registers to compensate.

I had a discussion about this with Maciej. He suggested that this
behavious of binutils should be taken a look at but also that we rather
should remove the -msoft-float option from the kernel and I support his
view.
I did add -msoft-float in 6218cf4410cfce7bc7e89834e73525b124625d4c
[[MIPS] Always pass -msoft-float.] in 2006 because back then there was a
wave of bug reports from people attempting to use hard fp in the kernel
which of course did result in FPR corruption. Adding -msoft-float made
sure that floating point operations would result in a link error because
the kernel does not supply a soft-fp library.
But maybe there are other methods to achieve the same - such as
#define float diediedie
#define double goboom

I had a short discussion with Matthew (cc-ed) meanwhile who has been
doing binutils and GCC work in this area recently and made GCC pass
`-msoft-float' down to GAS as a part of floating-point ABI updates being
made right now. As a result at this point I think we want to keep
`-msoft-float', and furthermore the use of `-Wa,-msoft-float' and `.set
hardfloat' appears mostly right to me however with the following
exceptions:

1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are available
(a single check will do, they were added to GAS both at the same time)
and only enable them if supported by binutils being used to build the
kernel, e.g. (for the `.set' part):

#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif

Otherwise we'd have to bump the binutils requirement up to 2.19; this
feature was only added with:

commit 037b32b9ffec4d7e68c596a0835dee8b0d26818f
Author: Adam Nemet <***@caviumnetworks.com>
Date: Mon Apr 28 17:06:28 2008 +0000

I'm not convinced that would be very wise, but maybe it's OK after six
years after all.

2. Use `.set hardfloat' only around the places that really require it,
i.e.:

.set push
SET_HARDFLOAT
# Do the FP stuff.
.set pop

(so the arch/mips/kernel/r4k_fpu.S piece is good except for maybe using
a macro, depending on the outcome of #1 above, but the other ones are
not).

So the patch looks to me like a good starting point, but it is not quite
there yet.

The use of `-Wa,-msoft-float' will also improve our safety checks with
GCC versions up to 4.9 as it'll catch any unintended use of FP operations
in assembly code as long as the version of GAS used is at least 2.19.

Maciej

[Manuel Lauss]

Post by Maciej W. Rozycki
1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are available
(a single check will do, they were added to GAS both at the same time)
and only enable them if supported by binutils being used to build the
#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif
Otherwise we'd have to bump the binutils requirement up to 2.19; this

Do people really update their toolchain so rarely?

Post by Maciej W. Rozycki
2. Use `.set hardfloat' only around the places that really require it,
.set push
SET_HARDFLOAT
# Do the FP stuff.
.set pop
(so the arch/mips/kernel/r4k_fpu.S piece is good except for maybe using
a macro, depending on the outcome of #1 above, but the other ones are
not).

I'll update the patch.

Thank you!
Manuel

[Ralf Baechle]

Post by Manuel Lauss

Post by Maciej W. Rozycki
Otherwise we'd have to bump the binutils requirement up to 2.19; this

Do people really update their toolchain so rarely?

The users of very old toolchains mostly fall into two categories:

- build farms. All that matters is if the code is building as it probably
won't ever get to see a CPU from the inside.
- users running into issues with an old kernel in an otherwise well
running system. They will try to use whatever is installed because
an upgrade can be technically hard - or somebody most likely wearing a
tie may throw a tantrum at the thought of upgrding.

Ralf

[Maciej W. Rozycki]

Post by Manuel Lauss

Post by Maciej W. Rozycki
1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are available
(a single check will do, they were added to GAS both at the same time)
and only enable them if supported by binutils being used to build the
#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif
Otherwise we'd have to bump the binutils requirement up to 2.19; this

Do people really update their toolchain so rarely?

I don't know, but unless they're toolchain developers at the same time
I'd expect some to stick with whatever they've found working. The worst
thing that can happen to you is when you need to upgrade the kernel to fix
a critical bug, then the updated kernel requires newer tools and then the
newer tools trigger a bunch of new bugs that you don't even know if they
are kernel or toolchain bugs (or both). So I don't want to force people
to upgrade unless absolutely necessary (e.g. a microMIPS kernel), I'd
rather let them do it whenever *they* feel comfortable doing it.

Linux's generic requirement is binutils 2.12 or newer, I reckon we bumped
the corresponding requirement for the MIPS port up a bit recently because
of some braindamage in binutils 2.24 the workaround for which has some
version limitations. And I am not convinced it is a good idea to bump the
requirement in such a short time again just because a GCC version to be
released next year have become strictier about the FP ABI (that we don't
use anyway). Especially as the solution is so simple.

I'm still at 2.20.1 as far as the MIPS target is concerned BTW, I just
considered the time I'd have to spend on upgrading would be better spent
on sorting out the kernel issues I've had outstanding, and there's been
quite a bunch.

Maciej

[Matthew Fortune]

Post by Maciej W. Rozycki

Post by Manuel Lauss

Post by Maciej W. Rozycki
1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are

available

Post by Manuel Lauss

Post by Maciej W. Rozycki
(a single check will do, they were added to GAS both at the same

time)

Post by Manuel Lauss

Post by Maciej W. Rozycki
and only enable them if supported by binutils being used to build

the

Post by Manuel Lauss

Post by Maciej W. Rozycki
#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif
Otherwise we'd have to bump the binutils requirement up to 2.19;

this

Post by Manuel Lauss
Do people really update their toolchain so rarely?

I don't know, but unless they're toolchain developers at the same time
I'd expect some to stick with whatever they've found working. The worst
thing that can happen to you is when you need to upgrade the kernel to fix
a critical bug, then the updated kernel requires newer tools and then the
newer tools trigger a bunch of new bugs that you don't even know if they
are kernel or toolchain bugs (or both). So I don't want to force people
to upgrade unless absolutely necessary (e.g. a microMIPS kernel), I'd
rather let them do it whenever *they* feel comfortable doing it.

Rather than me give a bunch of comments here could someone confirm what
your (kernel perspective) opinion is of what should change?

Post by Maciej W. Rozycki
From what I can tell it seems OK for pre-existing kernel releases to not

build with new tools from time to time. Also, it seems OK for currently
maintained branches to need a fix as long as it results in code which
can be built with old and new tools. This was my assumption when doing
the binutils work, it is just unfortunate that Manuel found the issue
before the kernel guys at Imagination had got round to discussing this
topic on the kernel list.

As a slight aside. Please do retain -msoft-float on kernel builds it is the
only way to ensure that no floating-point instructions are used even if
you have no 'float' or 'double' types. MIPS GCC relies on a cost model
to avoid floating point registers being used in the absence of floating
point types but this is not a guarantee. I do plan on working on this area
but it is non-trivial to say the least.

Regards,
Matthew

[Markos Chandras]

Post by Manuel Lauss

Post by Maciej W. Rozycki
1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are available
(a single check will do, they were added to GAS both at the same time)
and only enable them if supported by binutils being used to build the
#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif
Otherwise we'd have to bump the binutils requirement up to 2.19; this

Do people really update their toolchain so rarely?

Post by Maciej W. Rozycki
2. Use `.set hardfloat' only around the places that really require it,
.set push
SET_HARDFLOAT
# Do the FP stuff.
.set pop
(so the arch/mips/kernel/r4k_fpu.S piece is good except for maybe using
a macro, depending on the outcome of #1 above, but the other ones are
not).

I'll update the patch.
Thank you!
Manuel

Hi Manual,

Just wanted to ping you about this patch. Do you intend to submit a new
version including all the feedback from Ralf, Maciej and Matthew?

[Markos Chandras]

Post by Markos Chandras

Post by Manuel Lauss

Post by Maciej W. Rozycki
1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are available
(a single check will do, they were added to GAS both at the same time)
and only enable them if supported by binutils being used to build the
#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif
Otherwise we'd have to bump the binutils requirement up to 2.19; this

Do people really update their toolchain so rarely?

Post by Maciej W. Rozycki
2. Use `.set hardfloat' only around the places that really require it,
.set push
SET_HARDFLOAT
# Do the FP stuff.
.set pop
(so the arch/mips/kernel/r4k_fpu.S piece is good except for maybe using
a macro, depending on the outcome of #1 above, but the other ones are
not).

I'll update the patch.
Thank you!
Manuel

Hi Manual,
Just wanted to ping you about this patch. Do you intend to submit a new
version including all the feedback from Ralf, Maciej and Matthew?

s/Manual/Manuel/ :)

sorry about that

[Manuel Lauss]

Hi,

I still indend to, I just have a lot of non-linux related things to do
currently.
And I still struggle to get softloat gcc-4.9 and glibc built with
binutils commit
351cdf24d223290b15fa991e5052ec9e9bd1e284 applied, the linker throws float errors
left and right :(

Manuel

On Fri, Oct 10, 2014 at 4:40 PM, Markos Chandras

Post by Markos Chandras

Post by Markos Chandras

Post by Manuel Lauss

Post by Maciej W. Rozycki
1. Determine whether `-Wa,-msoft-float' and `.set hardfloat' are available
(a single check will do, they were added to GAS both at the same time)
and only enable them if supported by binutils being used to build the
#ifdef GAS_HAS_SET_HARDFLOAT
#define SET_HARDFLOAT .set hardfloat
#else
#define SET_HARDFLOAT
#endif
Otherwise we'd have to bump the binutils requirement up to 2.19; this

Do people really update their toolchain so rarely?

Post by Maciej W. Rozycki
2. Use `.set hardfloat' only around the places that really require it,
.set push
SET_HARDFLOAT
# Do the FP stuff.
.set pop
(so the arch/mips/kernel/r4k_fpu.S piece is good except for maybe using
a macro, depending on the outcome of #1 above, but the other ones are
not).

I'll update the patch.
Thank you!
Manuel

Hi Manual,
Just wanted to ping you about this patch. Do you intend to submit a new
version including all the feedback from Ralf, Maciej and Matthew?

s/Manual/Manuel/ :)
sorry about that
--
markos