mplayer: fix compilation of NEON assembly in libavcodec

Compilation of the NEON assembly fails unless neon is enabled in the
compiler options.  This is probably not needed for all combinations of
gcc/binutils, but at least some need it and it certainly doesn't hurt.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
 .../multimedia/mplayer/mplayer-1.1-fix-neon.patch   |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:

 Arnout> Compilation of the NEON assembly fails unless neon is enabled in the
 Arnout> compiler options.  This is probably not needed for all combinations of
 Arnout> gcc/binutils, but at least some need it and it certainly doesn't hurt.

 Arnout> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Arnout> ---
 Arnout>  .../multimedia/mplayer/mplayer-1.1-fix-neon.patch   |   19 +++++++++++++++++++
 Arnout>  1 file changed, 19 insertions(+)

 Arnout> diff --git a/package/multimedia/mplayer/mplayer-1.1-fix-neon.patch b/package/multimedia/mplayer/mplayer-1.1-fix-neon.patch
 Arnout> new file mode 100644
 Arnout> index 0000000..38c2adc
 Arnout> --- /dev/null
 Arnout> +++ b/package/multimedia/mplayer/mplayer-1.1-fix-neon.patch
 Arnout> @@ -0,0 +1,19 @@
 Arnout> +Compilation of the NEON assembly fails unless neon is enabled in the
 Arnout> +compiler options.  This is probably not needed for all combinations of
 Arnout> +gcc/binutils, but at least some need it and it certainly doesn't hurt.
 Arnout> +
 Arnout> +Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 Arnout> +---
 Arnout> +diff -Nrup mplayer-1.1.orig/configure mplayer-1.1/configure
 Arnout> +--- mplayer-1.1.orig/configure	2012-10-06 00:31:57.706985824 +0200
 Arnout> ++++ mplayer-1.1/configure	2012-10-06 00:49:32.618948116 +0200
 Arnout> +@@ -2935,6 +2935,9 @@ if arm ; then
 Arnout> +     inline_asm_check '"vadd.i16 q0, q0, q0"' && neon=yes
 Arnout> +   fi
 Arnout> +   echores "$neon"
 Arnout> ++  if test $neon = "yes" ; then
 Arnout> ++    extra_cflags="$extra_cflags -mfpu=neon -mfloat-abi=softfp"

Why softfp? Is that just an optimization or is it needed? How does that
work with a hardfp toolchain/libraries?

Please document why this is needed (if it is) in the commit message and
resubmit, thanks.

On 08/10/12 22:27, Peter Korsgaard wrote:
>   Arnout>  +diff -Nrup mplayer-1.1.orig/configure mplayer-1.1/configure
>   Arnout>  +--- mplayer-1.1.orig/configure	2012-10-06 00:31:57.706985824 +0200
>   Arnout>  ++++ mplayer-1.1/configure	2012-10-06 00:49:32.618948116 +0200
>   Arnout>  +@@ -2935,6 +2935,9 @@ if arm ; then
>   Arnout>  +     inline_asm_check '"vadd.i16 q0, q0, q0"'&&  neon=yes
>   Arnout>  +   fi
>   Arnout>  +   echores "$neon"
>   Arnout>  ++  if test $neon = "yes" ; then
>   Arnout>  ++    extra_cflags="$extra_cflags -mfpu=neon -mfloat-abi=softfp"
>
> Why softfp? Is that just an optimization or is it needed? How does that
> work with a hardfp toolchain/libraries?
>
> Please document why this is needed (if it is) in the commit message and
> resubmit, thanks.

  neon fpu (like vfp* fpu) can only be used with the softfp abi, so both are
needed.  This is kind of standard ARM knowledge :-)  Admittedly, it's silly
of gcc that it doesn't automatically set the float-abi to the only supported
one...

  Toolchains that hardcode -mfloat-abi=hard cannot have neon instructions,
I suppose.  I haven't tested it though.

  I also don't know what happens with an external toolchain that has
BR2_VFP_FLOAT set, because that option adds an -mfpu=vfp to the toolchain
command line.

  Regards,
  Arnout

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> Please document why this is needed (if it is) in the commit message and
 >> resubmit, thanks.

 Arnout>  neon fpu (like vfp* fpu) can only be used with the softfp abi,
 Arnout> so both are needed.  This is kind of standard ARM knowledge :-)

It is? I've never done any real work on platforms with neon, so it's
certainly a bit fussy to me, but I don't understand what the relation is
with the calling convention.

Aren't the new linaro toolchains hardfp and support neon?

Googling a bit I see stuff like:

http://wiki.meego.com/ARM/hardfp
http://sources.redhat.com/ml/crossgcc/2012-07/msg00091.html

http://wiki.debian.org/ArmHardFloatPort#GCC_floating-point_options
mentions that neon + hardfp abi isn't supported by GCC 4.4, but
otherwise doesn't mention that it cannot work.

>>>>> "Peter" == Peter Korsgaard <jacmet@uclibc.org> writes:

Hi Arnout,

 >>> Please document why this is needed (if it is) in the commit message and
 >>> resubmit, thanks.

 Arnout> neon fpu (like vfp* fpu) can only be used with the softfp abi,
 Arnout> so both are needed.  This is kind of standard ARM knowledge :-)

 Peter> It is? I've never done any real work on platforms with neon, so it's
 Peter> certainly a bit fussy to me, but I don't understand what the relation is
 Peter> with the calling convention.

 Peter> Aren't the new linaro toolchains hardfp and support neon?

 Peter> Googling a bit I see stuff like:

 Peter> http://wiki.meego.com/ARM/hardfp
 Peter> http://sources.redhat.com/ml/crossgcc/2012-07/msg00091.html

 Peter> http://wiki.debian.org/ArmHardFloatPort#GCC_floating-point_options
 Peter> mentions that neon + hardfp abi isn't supported by GCC 4.4, but
 Peter> otherwise doesn't mention that it cannot work.

Any comments on this?

On 15/10/12 08:53, Peter Korsgaard wrote:
>   Peter>  It is? I've never done any real work on platforms with neon, so it's
>   Peter>  certainly a bit fussy to me, but I don't understand what the relation is
>   Peter>  with the calling convention.
>
>   Peter>  Aren't the new linaro toolchains hardfp and support neon?
>
>   Peter>  Googling a bit I see stuff like:
>
>   Peter>  http://wiki.meego.com/ARM/hardfp
>   Peter>  http://sources.redhat.com/ml/crossgcc/2012-07/msg00091.html
>
>   Peter>  http://wiki.debian.org/ArmHardFloatPort#GCC_floating-point_options
>   Peter>  mentions that neon + hardfp abi isn't supported by GCC 4.4, but
>   Peter>  otherwise doesn't mention that it cannot work.
>
> Any comments on this?

  Sorry, I was going to test it with a Linaro toolchain but forgot about it.
Running it now.

  Regards,
  Arnout

On 10/10/12 11:13, Peter Korsgaard wrote:
>   Arnout>   neon fpu (like vfp* fpu) can only be used with the softfp abi,
>   Arnout>  so both are needed.  This is kind of standard ARM knowledge:-)
>
> It is? I've never done any real work on platforms with neon, so it's
> certainly a bit fussy to me, but I don't understand what the relation is
> with the calling convention.
>
> Aren't the new linaro toolchains hardfp and support neon?

  So these are my conclusions.

- Neon with hard float abi is indeed possible - teaches me not to trust the
gcc manpages :-)  BTW, the float abi is not just the function calling convention,
but also how float variables are laid out in memory - that's why it has an
impact on assembly code as well.

- That means, you need mfloat-abi=softfp on soft abi libc (e.g. sourcery) and
mfloat-abi=hard on hard abi libc (e.g. linaro).  It will be tough for buildroot
to distinguish between them and select the right -mfloat-abi.  On Linaro,
-mfloat-abi=hard is the default so it can be left out, but on Sourcery,
-mfloat-abi=softfp is required (default is soft).

- Internal toolchains are soft abi unless
BR2_EXTRA_GCC_CONFIG_OPTIONS="--with-float=hard" so they also need the
-mfloat-abi=softfp

- So we could just document in the external toolchains which options should be
given to enable neon.

- The vadd.i16 test in mplayer's configure script would detect that neon is not
available (because the -mfloat-abi and -mfpu options are not in CFLAGS) --
except that the test is disabled because we explicitly specify --enable-neon.

- ffmpeg itself always does the test, even if we give --enable-neon.


  So I propose to document the NEON options in the external toolchains, and to
remove --enable-neon from mplayer.  I'm not sure what to do with internal
and crosstool-ng toolchains.


  Regards,
  Arnout