Last Comment Bug 696291 - Support ARM soft-float in JM
: Support ARM soft-float in JM
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: ARM All
: -- normal with 2 votes (vote)
: mozilla12
Assigned To: David Anderson [:dvander]
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 697205 712261
  Show dependency treegraph
Reported: 2011-10-20 18:30 PDT by Andreas Gal :gal
Modified: 2012-01-31 06:52 PST (History)
19 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

wip v0 (17.10 KB, patch)
2011-10-21 00:34 PDT, David Anderson [:dvander]
no flags Details | Diff | Splinter Review
wip v1 (18.15 KB, patch)
2011-10-21 12:14 PDT, David Anderson [:dvander]
no flags Details | Diff | Splinter Review
v2 (18.73 KB, patch)
2011-10-27 10:33 PDT, David Anderson [:dvander]
no flags Details | Diff | Splinter Review
follow-up ARM fix (659 bytes, patch)
2012-01-18 17:39 PST, David Anderson [:dvander]
no flags Details | Diff | Splinter Review
follow-up ARM fix, attached correctly (662 bytes, patch)
2012-01-18 17:59 PST, David Anderson [:dvander]
marty.rosenberg: review+
Details | Diff | Splinter Review
ARM fix, v2 (1.10 KB, patch)
2012-01-20 18:31 PST, David Anderson [:dvander]
marty.rosenberg: review+
Details | Diff | Splinter Review

Description Andreas Gal :gal 2011-10-20 18:30:50 PDT
A significant fraction of Android phones shipping today still don't have a hardware FPU. We should urgently add soft-float support to JM so we can ship the native frontend for a wider range of Android phones. B2G wants this for lower-end hardware as well.
Comment 1 David Anderson [:dvander] 2011-10-21 00:34:15 PDT
Created attachment 568618 [details] [diff] [review]
wip v0

Disabling FPU support, down to a few test failures on jit-tests
Comment 2 David Anderson [:dvander] 2011-10-21 12:14:06 PDT
Created attachment 568734 [details] [diff] [review]
wip v1

passes jit-tests and reftests
Comment 3 David Mandelin [:dmandelin] 2011-10-25 13:30:55 PDT
Ted, IIUC, you now have the ball, meaning that you're going to get some ARMv6 builds going and try this patch with them. If that's not right, please let us know right away so we can keep this moving.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-25 16:24:06 PDT
Sayrer and I wrote up instructions for running what were formerly known as trace-tests (forget the new name) here: .  That's probably the easiest and most debuggable way to test jseng on android devices.  For some reason those instructions were removed from the current android wiki page, but they should still work (module trace-tests rename).

Andreas and I brought back some crappy phones from China among which should be some ARMv6 chips.  He should have them in the MV office.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-10-27 10:32:26 PDT
dmandelin: sorry, I wasn't CCed here, but yeah, I'm driving this. I'll figure out how to test this patch.

cjones: dougt gave me one of those China Mobile phones, but it's ARMv5TE. :-/ Are there other ones that are ARMv6?
Comment 6 David Anderson [:dvander] 2011-10-27 10:33:53 PDT
Created attachment 570021 [details] [diff] [review]
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-27 11:15:38 PDT
Not sure, they don't live in my city.  gal, dougt?
Comment 8 Andreas Gal :gal 2011-10-27 11:18:02 PDT
Ted, I have 4 phones in my hand. All of them are v6 I believe. Do you want the physical phones (overnight fedex), or should I attach them to a network connected machine via usb and you take it from there?
Comment 9 Marco Castelluccio [:marco] 2012-01-06 09:40:19 PST
I've an armv6 phone, I can do tests if you need.
Comment 10 :Ms2ger (⌚ UTC+1/+2) 2012-01-18 02:53:00 PST
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-18 11:30:34 PST
This bug seems to have regressed SunSpider on Android by ~50%

Regression SunSpider NoChrome increase 57.2% on Android 2.2 mobile
    Previous: avg 75.335 stddev 0.796 of 30 runs up to revision f4049f65efc6
    New     : avg 118.408 stddev 1.143 of 5 runs since revision 7538f4d4697c
    Change  : +43.073 (57.2% / z=54.146)
    Graph   :

Changeset range:

The notification is from mozilla-central, so I looked on mozilla-inbound. This patch is where the regression occurred.
Comment 12 David Anderson [:dvander] 2012-01-18 11:57:37 PST
Hrm, this probably means the VFP detection is broken.
Comment 13 Marco Castelluccio [:marco] 2012-01-18 16:19:36 PST
Is the code here correct?
WTF_CPU_ARM_THUMB2 is always set to 0, also when WTF_THUMB_ARCH_VERSION is 4 (ARMv7, that has thumb2) and WTF_CPU_ARM_TRADITIONAL is always set to 1.

However I've written a function to read CPU flags, it could be used to check whatever cpu flag (for example neon, vfp, vfpv3, thumb):

static bool isVFPPresent() {
    ifstream file("/proc/cpuinfo");
    string flagsLine;

    while(getline(file, flagsLine))
      if(!,5,"flags") && flagsLine.find("vfp") != string::npos)
        return true;

  return false;
Comment 14 David Anderson [:dvander] 2012-01-18 17:39:00 PST
Created attachment 589731 [details] [diff] [review]
follow-up ARM fix

as far as I can tell, this function was just returning false on ARM.
Comment 15 David Anderson [:dvander] 2012-01-18 17:59:58 PST
Created attachment 589741 [details] [diff] [review]
follow-up ARM fix, attached correctly
Comment 16 David Anderson [:dvander] 2012-01-18 18:13:00 PST
Comment 17 Phil Ringnalda (:philor) 2012-01-18 20:38:18 PST
Backed out in - that doesn't actually build on Android.
Comment 18 Andreas Gal :gal 2012-01-18 20:53:10 PST
Very excited to see activity in this bug. Please keep it rolling :)
Comment 19 David Anderson [:dvander] 2012-01-20 15:41:17 PST
I've pushed a fix to try that is mostly stolen from pixman. If it works I'll r? marty again.
Comment 20 David Anderson [:dvander] 2012-01-20 18:31:06 PST
Created attachment 590405 [details] [diff] [review]
ARM fix, v2

passed on try
Comment 21 Marty Rosenberg [:mjrosenb] 2012-01-20 18:41:16 PST
Comment on attachment 590405 [details] [diff] [review]
ARM fix, v2

Review of attachment 590405 [details] [diff] [review]:

Good to hear this one is green on try.
Comment 22 David Anderson [:dvander] 2012-01-30 18:56:24 PST
Whoops, apparently I never landed this!
Comment 23 Ed Morley [:emorley] 2012-01-31 06:52:32 PST

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