Re: [PATCH 1 of 1] gcc: support 'softfp' toolchains for ARM

["Yann E. MORIN" <yann dot morin dot 1998 at anciens dot enib dot fr>]

Javier, BenoÃt, All,

On Wednesday 31 August 2011 21:37:21 BenoÃt THÃBAUDEAU wrote:
> > gcc: support 'softfp' toolchains for ARM
> >
> > Allows to build toolchains with default '-mfloat-abi=softfp
> > -mfpu=...'.
> 
> Good idea.

Yes, pretty rad! :-)

> Until now, I used to add extra config options when I needed softfp on
> ARM.
> 
> But you handle only the softfp case, although the hard case also does not
> produce any options (only the soft case did before your patch).

Let's handle things one by one. Pure hard FP can be addressed in a
separate patch.

> Do you think the
> hard case would be worth adding, or should the softfp/hard choice be left up to
> the toolchain defaults if ARCH_FLOAT_HW=y?

We should not rely on some internal black-magic-that-can-change-between-
versions-or-the-phase-of-the-moon to decide what to do. Let's be explicit
about it, even if redondant. Let's just do it right. ;-)

> I wonder if the softfp/hard case had
> not been dropped on purpose by Yann to avoid some issues on non-ARM targets.

Well, I mostly target pure soft float, as my machines do not have an FPU (in
fact, my beagleboard has, but I have not yet had time to investigate...)

And non-ARM should just behave.

[--SNIP--]
> +config ARCH_SUPPORT_SOFTFP
> +    bool
> +    default n

'default n' is not needed. 'n' *is* the default.

> [*---SNIP---*]
> > diff -r a171c274bc12 -r 3d480b953bf8 config/target.in
> > --- a/config/target.in	Tue Aug 30 19:38:10 2011 +0200
> > +++ b/config/target.in	Wed Aug 31 17:55:12 2011 +0200
> [*---SNIP---*]
> > @@ -271,6 +277,11 @@
> >        If your processor has no FPU, then you most probably want
> >        this, as it
> >        is faster than emulating the FPU in the kernel.
> >
> > +config ARCH_FLOAT_SOFTFP
> > +    bool
> > +    prompt "softfp"
> > +    depends on ARCH_SUPPORT_SOFTFP
> > +
> >  endchoice
> >
> >  config TARGET_CFLAGS
> 
> Some help message would probably be useful for this option, something close to
> the help message of ARCH_FLOAT_HW, stressing the fact that only the float ABI
> will differ from the true hard option (which is not necessarily ARCH_FLOAT_HW
> depending on the toolchain defaults as I explained above).

Yes, indeed. And this page is very interesting to read:
http://wiki.debian.org/ArmHardFloatPort/VfpComparison#Details_on_GCC_floating-point_options

Also: I would not put it in the choice. In my opinion, 'softfp' is just
one kind of hard-float. It's an ARM-only option. Eg., smthg like:

config/arch/arm.in.2:
config ARCH_ARM_SOFTFP
    bool
    prompt "Use 'softfp'"
    depends on ARCH_FLOAT_HARD

> > diff -r a171c274bc12 -r 3d480b953bf8 scripts/build/arch/arm.sh
> > --- a/scripts/build/arch/arm.sh	Tue Aug 30 19:38:10 2011 +0200
> > +++ b/scripts/build/arch/arm.sh	Wed Aug 31 17:55:12 2011 +0200
> > @@ -4,6 +4,9 @@
> >      # The architecture part of the tuple:
> >      CT_TARGET_ARCH="${CT_ARCH}${target_endian_eb}"
> >
> > +    # gcc ./configure flags
> > +    [ "${CT_ARCH_FLOAT_SOFTFP}" ] && {
> > CT_ARCH_FLOAT_CFLAG="-mfloat-abi=softfp";
> > CT_ARCH_WITH_FLOAT="--with-float=softfp"; }
> > +

Such contructs are hard to maintain in the long run. Please use:
  if [ ...]; then
    ...
  fi

Yeah, I know there are a bunch of them in the code. I leave them alone, but
as soon I need to tweak them, they get rewritten of if-then[-else]-fi. Takes
more lines, but easier to read/understand/maintain/fix.

> Why not moving that to scripts/functions, below CT_ARCH_FLOAT_SW,

If we go with 'softfp' as a pure ARM option (as suggested above), then the
comment is moot.

If 'softfp' is deemed a generic option, then it should go to the generic
script, indeed.

> as the softfp
> option may be supported on other targets in the future?
> Do you think the writing of the softfp choice would differ between targets?

I don't know about a target that supports softfp. Either they have an FPU,
or none at all. I don't think any architecture but ARM supports softfp...

> > diff -r a171c274bc12 -r 3d480b953bf8
> > scripts/build/libc/glibc-eglibc.sh-common
> > --- a/scripts/build/libc/glibc-eglibc.sh-common	Tue Aug 30 19:38:10
> > 2011 +0200
> > +++ b/scripts/build/libc/glibc-eglibc.sh-common	Wed Aug 31 17:55:12
> > 2011 +0200
> > @@ -132,9 +132,9 @@
> >          *) extra_config+=("--disable-shared");;
> >      esac
> >
> > -    case "${CT_ARCH_FLOAT_HW},${CT_ARCH_FLOAT_SW}" in
> > -        y,) extra_config+=("--with-fp");;
> > -        ,y) extra_config+=("--without-fp");;
> > +    case
> > "${CT_ARCH_FLOAT_HW},${CT_ARCH_FLOAT_SW},${CT_ARCH_FLOAT_SOFTFP}" in
> > +        y,,) extra_config+=("--with-fp");;
> > +        ,y,|,,y) extra_config+=("--without-fp");;
> >      esac
> >
> >      if [ "${CT_LIBC_DISABLE_VERSIONING}" = "y" ]; then
> 
> I think that's wrong. I have looked at eglibc's sources, and all it does with
> "--with-fp" is use floating-point types. Hence, as softfp uses the FPU
> opcodes, it would be a shame not to benefit from them here.
> 
> Or did you do that on purpose in order to avoid ABI compatibility issues?
> softfp is much less interesting if the libc does not use it everywhere it
> can. The only thing to make sure here is that the libc will not produce
> hard-float ABI code if softfp is selected and "--with-fp" is used, which
> could happen if there are hand-written assembly files in the libc.

Yeah. If we treat 'softfp' as a kind of hard-float (as suggested above),
then this should work, right?

> > diff -r a171c274bc12 -r 3d480b953bf8 scripts/build/libc/uClibc.sh
> > --- a/scripts/build/libc/uClibc.sh	Tue Aug 30 19:38:10 2011 +0200
> > +++ b/scripts/build/libc/uClibc.sh	Wed Aug 31 17:55:12 2011 +0200
> > @@ -352,13 +352,13 @@
> >
> >      # Accomodate for old and new uClibc version, where the
> >      # way to select between hard/soft float has changed
> > -    case "${CT_ARCH_FLOAT_HW},${CT_ARCH_FLOAT_SW}" in
> > -        y,) cat <<-ENDSED
> > +    case
> > "${CT_ARCH_FLOAT_HW},${CT_ARCH_FLOAT_SW},${CT_ARCH_FLOAT_SOFTFP}" in
> > +        y,,) cat <<-ENDSED
> >  				s/^[^_]*(HAS_FPU).*/\\1=y/
> >  				s/.*(UCLIBC_HAS_FPU).*/\\1=y/
> >  				ENDSED
> >              ;;
> > -        ,y) cat <<-ENDSED
> > +        ,y,|,,y) cat <<-ENDSED
> >  				s/^[^_]*(HAS_FPU).*/\\# \\1 is not set/
> >  				s/.*(UCLIBC_HAS_FPU).*/# \\1 is not set/
> >  				ENDSED
> 
> I don't know what uClibc does with this option, but the same comment as
> above for the libc applies here.

Yep.

> Apart from that, sounds good to me.

Yes! Pretty neat, we have a working starting point to work on! Yeah! \o/
Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

--
For unsubscribe information see http://sourceware.org/lists.html#faq