1360429 - Support NEON (support_neon) on aarch64

Status: RESOLVED FIXED

(Core :: mozglue, enhancement)

Description

[Makoto Kato [:m_kato]]

aarch64 supports NEON, but supports_neon doesn't return true .

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Bug 1360429 - Part 1. inline arm assember for YCBCR should be on arm only.

ycvcr_to_rgb565 uses inline assember for arm neon.  Since it is different for aarch64's assembler, we should define HAVE_YCBCR_TO_RGB565 on arm32 only.

Review commit: https://reviewboard.mozilla.org/r/134586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134586/

Comment 2

[Makoto Kato [:m_kato]]

Attachment: Bug 1360429 - Part 2. Detect __ARM_NEON macro for aarch64.

aarch64's gcc defines __ARM_NEON for NEON, so we should detect it to support NEON code.

Review commit: https://reviewboard.mozilla.org/r/134588/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134588/

Comment 3

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8862722 [details]
Bug 1360429 - Part 2. Detect __ARM_NEON macro for aarch64.

https://reviewboard.mozilla.org/r/134588/#review137666

Comment 4

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8862721 [details]
Bug 1360429 - Part 1. inline arm assember for YCBCR should be on arm only.

https://reviewboard.mozilla.org/r/134586/#review137668

Comment 5

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8862722 [details]
Bug 1360429 - Part 2. Detect __ARM_NEON macro for aarch64.

https://reviewboard.mozilla.org/r/134588/#review137670

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8862722 [details]
Bug 1360429 - Part 2. Detect __ARM_NEON macro for aarch64.

https://reviewboard.mozilla.org/r/134588/#review140434

::: mozglue/build/arm.h:81
(Diff revision 1)
>  
>  #endif
>  
> +// When using -mfpu=neon on arm, or using default on aarch64,
> +// gcc generates neon instructions.
> +#if defined(__GNUC__) && (defined(__ARM_NEON__) || defined(__ARM_NEON))

No real need for __GNUC__, and all compilers that define __ARM_NEON__ also define __ARM_NEON, so you can limit to the latter.

Comment 7

[Makoto Kato [:m_kato]]

"__ARM_NEON__" isn't defined on aarch64 compiler.

(4.9 for android)
aarch64-linux-android-cpp -dM < /dev/null | grep NEON
#define __ARM_NEON 1

arm-linux-androideabi-cpp -dM -mfpu=neon -mfloat-abi=softfp -march=armv7-a < /dev/null | grep NEON
#define __ARM_NEON_FP 4
#define __ARM_NEON__ 1
#define __ARM_NEON 1

(5.4 on Ubuntu 16.04TLS)
aarch64-linux-gnu-cpp -dM </dev/null | grep NEON
#define __ARM_NEON 1

arm-linux-gnueabihf-cpp -march=armv7-a -mfloat-abi=softfp -mfpu=neon -dM < /dev/null | grep NEON
#define __ARM_NEON_FP 4
#define __ARM_NEON__ 1
#define __ARM_NEON 1

Comment 8

[Makoto Kato [:m_kato]]

Comment on attachment 8862722 [details]
Bug 1360429 - Part 2. Detect __ARM_NEON macro for aarch64.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134588/diff/1-2/

Comment 9

[Mike Hommey [:glandium]]

You're only confirming what I wrote ;)

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8862722 [details]
Bug 1360429 - Part 2. Detect __ARM_NEON macro for aarch64.

https://reviewboard.mozilla.org/r/134588/#review141964

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44e7fc73aeb5
Part 1. inline arm assember for YCBCR should be on arm only. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/edea8382d3f4
Part 2. Detect __ARM_NEON macro for aarch64. r=glandium

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/44e7fc73aeb5
https://hg.mozilla.org/mozilla-central/rev/edea8382d3f4