Support ARM soft-float in JM

RESOLVED FIXED in mozilla12

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gal, Assigned: dvander)

Tracking

Trunk
mozilla12
ARM
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 568618 [details] [diff] [review]
wip v0

Disabling FPU support, down to a few test failures on jit-tests
(Assignee)

Comment 2

6 years ago
Created attachment 568734 [details] [diff] [review]
wip v1

passes jit-tests and reftests
(Assignee)

Updated

6 years ago
Attachment #568618 - Attachment is obsolete: true
Blocks: 697205
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.
Sayrer and I wrote up instructions for running what were formerly known as trace-tests (forget the new name) here: https://wiki.mozilla.org/index.php?title=Mobile/Fennec/Android&oldid=257866#JS_trace-tests .  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.
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?
(Assignee)

Comment 6

6 years ago
Created attachment 570021 [details] [diff] [review]
v2
Assignee: general → dvander
Attachment #568734 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Not sure, they don't live in my city.  gal, dougt?
(Reporter)

Comment 8

6 years ago
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?
Blocks: 712261
I've an armv6 phone, I can do tests if you need.
https://hg.mozilla.org/mozilla-central/rev/095649e65552
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
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   : http://mzl.la/AuWA0S

Changeset range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f4049f65efc6&tochange=7538f4d4697c


The notification is from mozilla-central, so I looked on mozilla-inbound. This patch is where the regression occurred.
(Assignee)

Comment 12

6 years ago
Hrm, this probably means the VFP detection is broken.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Is the code here correct? https://mxr.mozilla.org/mozilla-central/source/js/src/assembler/wtf/Platform.h#340
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() {
  #if WTF_PLATFORM_LINUX
    ifstream file("/proc/cpuinfo");
    string flagsLine;

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

  return false;
}
(Assignee)

Comment 14

6 years ago
Created attachment 589731 [details] [diff] [review]
follow-up ARM fix

as far as I can tell, this function was just returning false on ARM.
Attachment #589731 - Flags: review?(mrosenberg)
(Assignee)

Comment 15

6 years ago
Created attachment 589741 [details] [diff] [review]
follow-up ARM fix, attached correctly
Attachment #589731 - Attachment is obsolete: true
Attachment #589731 - Flags: review?(mrosenberg)
Attachment #589741 - Flags: review?(mrosenberg)
Attachment #589741 - Flags: review?(mrosenberg) → review+
(Assignee)

Comment 16

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/78a8aeae5b30
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/79b026da30f0 - that doesn't actually build on Android.
(Reporter)

Comment 18

6 years ago
Very excited to see activity in this bug. Please keep it rolling :)
Target Milestone: mozilla12 → ---
(Assignee)

Comment 19

6 years ago
I've pushed a fix to try that is mostly stolen from pixman. If it works I'll r? marty again.
(Assignee)

Comment 20

6 years ago
Created attachment 590405 [details] [diff] [review]
ARM fix, v2

passed on try
Attachment #589741 - Attachment is obsolete: true
Attachment #590405 - Flags: review?(mrosenberg)
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.
Attachment #590405 - Flags: review?(mrosenberg) → review+
(Assignee)

Updated

6 years ago
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 22

6 years ago
Whoops, apparently I never landed this!

http://hg.mozilla.org/integration/mozilla-inbound/rev/0aacfba4caab
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/0aacfba4caab
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.