Last Comment Bug 667130 - SIMD/THUMB2 detections fail under Android NDK5
: SIMD/THUMB2 detections fail under Android NDK5
Status: RESOLVED FIXED
: mobile, perf
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: ARM Linux
: -- normal (vote)
: mozilla7
Assigned To: Brad Lassey [:blassey] (use needinfo?)
:
: Gregory Szorc [:gps]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-24 18:36 PDT by Jim Chen [:jchen] [:darchons]
Modified: 2011-06-30 05:49 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
wip patch (1.76 KB, text/plain)
2011-06-24 18:36 PDT, Jim Chen [:jchen] [:darchons]
no flags Details
patch to fix simd/thumb2 detection (818 bytes, patch)
2011-06-29 12:04 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
patch to fix simd/thumb2 detection (826 bytes, patch)
2011-06-29 12:21 PDT, Brad Lassey [:blassey] (use needinfo?)
mh+mozilla: review+
Details | Diff | Splinter Review

Description Jim Chen [:jchen] [:darchons] 2011-06-24 18:36:07 PDT
Created attachment 541881 [details]
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) 2011-06-28 18:33:37 PDT
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?) 2011-06-28 20:32:35 PDT
(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) 2011-06-28 21:03:09 PDT
Issue 2 is a dupe of bug 666918.
Comment 4 Jacob Bramley [:jbramley] 2011-06-29 08:29:35 PDT
(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) 2011-06-29 11:09:47 PDT
Let's refocus this bug on the SIMD/Thumb2 detection failure in NDK5.
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2011-06-29 12:04:02 PDT
Created attachment 542899 [details] [diff] [review]
patch to fix simd/thumb2 detection
Comment 7 Mike Hommey [:glandium] 2011-06-29 12:12:46 PDT
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?) 2011-06-29 12:16:00 PDT
(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?) 2011-06-29 12:21:15 PDT
Created attachment 542906 [details] [diff] [review]
patch to fix simd/thumb2 detection
Comment 10 Mike Hommey [:glandium] 2011-06-29 12:26:55 PDT
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?) 2011-06-29 12:32:52 PDT
pushed to inbound (m-c is pretty colors right now) http://hg.mozilla.org/integration/mozilla-inbound/rev/1b9de1c16455
Comment 12 Marco Bonardo [::mak] 2011-06-30 05:49:28 PDT
http://hg.mozilla.org/mozilla-central/rev/1b9de1c16455

Note You need to log in before you can comment on or make changes to this bug.