667130 - SIMD/THUMB2 detections fail under Android NDK5

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: wip patch

1) HAVE_ARM_SIMD and HAVE_ARM_THUMB2 are not detected in configure because we detect them based on target, and for NDK5 we are using a different target in mozconfig

2) When building with -fno-omit-frame-pointer, "gfx/ycbcr/yuv_convert_arm.cpp" and "toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc" fail for using the frame pointer register. "exception_handler.cc" has protection for this (see toolkit/crashreporter/google-breakpad/src/client/linux/handler/Makefile.in) but it didn't work for me when I try something like 'ac_add_options --enable-optimize="-fno-omit-frame-pointer"'

Attached is the WIP patch I used to fix the -fno-omit-frame-pointer bug

Comment 1

[Alon Zakai (:azakai)]

Some questions:

Did you see any problem with 1), or just notice it?

Is issue 2) new with NDK5, or existing with NDK4 as well?

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #1)
> Some questions:
> 
> Did you see any problem with 1), or just notice it?
In the very least, this causes us to not build pixman-arm-simd.c and pixman-arm-simd-asm.S.

> Is issue 2) new with NDK5, or existing with NDK4 as well?
this is only a problem with NDK5

Comment 3

[Timothy B. Terriberry (:derf)]

Issue 2 is a dupe of bug 666918.

Comment 4

[Jacob Bramley [:jbramley]]

(In reply to comment #2)
> (In reply to comment #1)
> > Is issue 2) new with NDK5, or existing with NDK4 as well?
> this is only a problem with NDK5

I think issue 2 will turn up in NDK5 once issue 1 is fixed. The error is thrown from a NEON routine, so I assume that it's just not being compiled under NDK5, since it thinks it doesn't have NEON.

I've attached a fix for issue 2 to bug 666918.

Comment 5

[Alon Zakai (:azakai)]

Let's refocus this bug on the SIMD/Thumb2 detection failure in NDK5.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch to fix simd/thumb2 detection

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 542899 [details] [diff] [review]
patch to fix simd/thumb2 detection

Review of attachment 542899 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +6959,4 @@
>  
>  dnl Defaults
>  case "${target}" in
> + *-android*|*-linuxandroid*)

You could use *-*android*) ... but why remove arm?

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #7)
> Comment on attachment 542899 [details] [diff] [review] [review]
> You could use *-*android*) ... 
yup, but in previous patches/reviews we decided to be more prescriptive 
> but why remove arm?
probably shouldn't

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch to fix simd/thumb2 detection

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 542906 [details] [diff] [review]
patch to fix simd/thumb2 detection

Review of attachment 542906 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +6959,4 @@
>  
>  dnl Defaults
>  case "${target}" in
> + arm-android*|arm-linuxandroid*)

r=me with that heading space removed.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

pushed to inbound (m-c is pretty colors right now) http://hg.mozilla.org/integration/mozilla-inbound/rev/1b9de1c16455

Comment 12

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/1b9de1c16455