[RFC] ARM -mfpu=auto woes

I'm looking into a regression between gcc-7 and gcc-8 that causes
compilation with -mfloat-abi=hard to fail on arm-eabi with:

$ arm-eabi-gcc -c -mfloat-abi=hard t.c
cc1: error: -mfloat-abi=hard: selected processor lacks an FPU

Per the documentation, -mfpu=auto was supposed to be the default, but
the implicit -mcpu=armv7tdmi option from configure_default_options, or
the -march=armv4t derived from it, seems to be taken as choosing to
disable the fpu.  Even passing -mfpu=auto explicitly one gets the error
above; AFAICT it would only work if the default cpu had support for an
fpu, and the "regression" is caused by new checks about conflicting
options.  Does that sound right?

While trying to find a work-around, still suspecting the issue was a
lack of -mfpu=auto, I tried configuring --with-fpu=auto and ran into a
few issues.


The first problem was a shell bug in the $fpu = error test: there must
be a blank before the closing bracket.


The second problem, hit after fixing the first, was that it printed a
very confusing error message:

  Unknown target in --with-arch=

which set me down a bit of a wild goose chase until I realized the error
was about with_fpu, but the error message, presumably copied from an
earlier loop, had not been adjusted for the fact that 'which' and 'val',
set in the loop, were not set in the block below, and we don't want to
print the values in the last iteration of the loop, we want the
--with-fpu stuff.


The third problem is that chkfpu rejects auto because auto is special,
it's not in the fpu_cnames array.  I'm not sure whether it's
inappropriate to accept it, but if we could accept it so as to enable it
to be an overriding default, it could be arranged to be explicitly
accepted right in the portion of config.gcc modified by the patch below,
or in parsecpu.awk.  Any preferences?

Weirdly, *without* the obvious fixes in the patch below, --with-fpu=auto
is accepted, but after the patch it's no longer accepted, so I guess it
might make sense to refrain from installing it until there's a decision
as to whether --with-fpu=auto should be accepted.  So, any objections to
my installing this then, or even now, if --with-fpu=auto shouldn't be
accepted?


for  gcc/ChangeLog

	* config.gcc: Fix ARM --with-fpu checking and error message.

On Jun 12, 2019, Alexandre Oliva <oliva@adacore.com> wrote:

> I'm looking into a regression between gcc-7 and gcc-8 that causes
> compilation with -mfloat-abi=hard to fail on arm-eabi with:

> $ arm-eabi-gcc -c -mfloat-abi=hard t.c
> cc1: error: -mfloat-abi=hard: selected processor lacks an FPU

> Per the documentation, -mfpu=auto was supposed to be the default, but
> the implicit -mcpu=armv7tdmi option from configure_default_options, or
> the -march=armv4t derived from it, seems to be taken as choosing to
> disable the fpu.

Or rather they don't have any fpu-related options, so none of the fpu
bits are set, and -mfpu=auto thus necessarily resolves to nofp.

What's confusing to me is that specifying -mfpu=vfp is accepted, even
though -mcpu=arm7tdmi+vfp isn't, and with that, -mfloat-abi=hard goes
through as before.

Would it make sense, for backward-compatibility purposes, to implicitly
enable vfpv2 for -mfpu=auto, even for CPUs not configured as having an
FP option, when given -mfloat-abi=hard?

Or is this error introduced in GCC 8.* actually desirable, and requiring
an explicit override of -mfpu, say =vfpv2 along for -mfloat-abi=hard,
when the default CPU, e.g. arv7tdmi, doesn't have an FP, something that
users are expected to do?

Thanks in advance,

On 13/06/2019 14:00, Alexandre Oliva wrote:
> On Jun 12, 2019, Alexandre Oliva <oliva@adacore.com> wrote:
> 
>> I'm looking into a regression between gcc-7 and gcc-8 that causes
>> compilation with -mfloat-abi=hard to fail on arm-eabi with:
> 
>> $ arm-eabi-gcc -c -mfloat-abi=hard t.c
>> cc1: error: -mfloat-abi=hard: selected processor lacks an FPU
> 
>> Per the documentation, -mfpu=auto was supposed to be the default, but
>> the implicit -mcpu=armv7tdmi option from configure_default_options, or
>> the -march=armv4t derived from it, seems to be taken as choosing to
>> disable the fpu.
> 
> Or rather they don't have any fpu-related options, so none of the fpu
> bits are set, and -mfpu=auto thus necessarily resolves to nofp.

Which is correct.  arm7tdmi implements architecture version armv4t, but
floating point wasn't added until armv5t, and in practice, we only
support it from armv5te when extra instructions were added to support
moving double precision values in and out of GP registers.  In practice,
not supporting FP on armv5t is not a real problem as I don't think any
v5t implementations with FP ever got into real products: availability of
FP in hardware did not really become widespread until we reached armv7.

The existing -mfpu support was a complete mess.  The compiler would
accept pretty much anything, even if it was completely meaningless.  You
could target combinations that simply did not exist, or you could
generate code that was sub-optimial simply because you had to specify it
all and specify it correctly in order to get the right output.

It gets worse, because the Arm architecture does not represent a linear
sequence of increasing capabilities.  The m-profile, which was created
around the time of armv7 introduced a new subsetting of the
architecture; and in particular the FP variants of that do not
necessarily support all the instructions that other, earlier FP releases
have.  In particular, it's not uncommon for m-profile cores to only
support single-precision floating-point operations.

When I added the new code I left the old way in, because I didn't want
to unduly break existing users; but neither did I want to leave in the
free-for all that we have today where we simply did things wrong
silently.  So -mfpu=auto uses the internal architectural tables to
validate the options against what is a permitted variant within the
architecture.  You can still generate sub-optimal code in some cases,
but you have to try harder and, most of the time, it will be less
sub-optimal than it was previously.

> 
> What's confusing to me is that specifying -mfpu=vfp is accepted, even
> though -mcpu=arm7tdmi+vfp isn't, and with that, -mfloat-abi=hard goes
> through as before.
> 
> Would it make sense, for backward-compatibility purposes, to implicitly
> enable vfpv2 for -mfpu=auto, even for CPUs not configured as having an
> FP option, when given -mfloat-abi=hard?

No.  If you want backwards compatibility, you can still force a specific
FPU via -mfpu=<name of fpu>, but the new way deliberately does not
follow that nightmare.  Note that -mfpu is essentially deprecated now;
new FPU variants will no-longer use it and at some point we will likely
obsolete and then delete it entirely, making -mfpu=auto the one and only
way.

So what you were asking for on the command line was meaningless.  It
went through and generated code, but it targetted a non-existent
architecture and you were hamstringing your code by doing that.  The old
interface would allow you to do that, the new doesn't; but that's not a
bad thing.

> 
> Or is this error introduced in GCC 8.* actually desirable, and requiring
> an explicit override of -mfpu, say =vfpv2 along for -mfloat-abi=hard,
> when the default CPU, e.g. arv7tdmi, doesn't have an FP, something that
> users are expected to do?
> 

Exactly, it's a good thing and was certainly a deliberate part of the
new design.

R.

> Thanks in advance,
>

Richard,

Thanks for your feedback.

I conclude from it that it's not worth it to introduce code to support
configuring --with-fpu=auto, so I'm going ahead and installing just the
obvious configury bits I'd posted before.

On Jun 12, 2019, Alexandre Oliva <oliva@adacore.com> wrote:

> So, any objections to my installing this then, or even now, if
> --with-fpu=auto shouldn't be accepted?

fix ARM --with-fpu option checking and error message

Fix the test for failure in parsecpu's checking of the --with-fpu
argument, and the error message that gets printed when the check
fails.

for  gcc/ChangeLog

	* config.gcc: Fix ARM --with-fpu checking and error message.

---
 gcc/config.gcc |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index fda048dc12b1..7119698779fd 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4002,12 +4002,13 @@ case "${target}" in
 
 		# see if --with-fpu matches any of the supported FPUs
 		if [ x"$with_fpu" != x ] ; then
+		  val=$with_fpu
 		  fpu=`awk -f ${srcdir}/config/arm/parsecpu.awk \
-			-v cmd="chkfpu $with_fpu" \
+			-v cmd="chkfpu $val" \
 			${srcdir}/config/arm/arm-cpus.in`
-		  if [ "$fpu" = "error"]
+		  if [ "$fpu" = "error" ]
 		  then
-		    echo "Unknown target in --with-$which=$val" 1>&2
+		    echo "Unknown target in --with-fpu=$val" 1>&2
 		    exit 1
 		  fi
 		fi