Closed
Bug 521152
Opened 15 years ago
Closed 15 years ago
TM: factorial computed incorrectly on ARM
Categories
(Core :: JavaScript Engine, defect, P2)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta3-fixed |
People
(Reporter: jruderman, Assigned: gal)
References
Details
(Keywords: testcase, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
695 bytes,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
These scripts incorrectly give -2102132736 instead of 2432902008176640000 when running with -j on ARM. Tested using rev 3a72a9e5ccd7+. Smells of int/double confusion. Related to bug 499889? Tail-recursive version: function factorial_tail(n, acc) { if (n == 0) { return acc; } return factorial_tail(n - 1, acc * n); } print(factorial_tail(20, 1)); Iterative version: var acc = 1; for (var i=1; i<=20; ++i) acc *= i; print(acc);
Assignee | ||
Updated•15 years ago
|
Assignee: general → gal
tracking-fennec: --- → ?
Assignee | ||
Comment 1•15 years ago
|
||
Great testcase. Should be an easy fix. Trying to reproduce.
Assignee | ||
Comment 2•15 years ago
|
||
Looks like ARM doesn't set the overflow flag on multiply. That is very sad.
Assignee | ||
Comment 3•15 years ago
|
||
004042afa8 mul r9,r10,r9 r0(state) r7(sp) r9(ld2) r10(ld3) ov1 = ov mul1 # codegen'd with the xt xt1: xt ov1 -> pc=0x2ba9a3 imacpc=(nil) sp+16 rp+0 (GuardID=002) 004042afac bvs 0x4042bed0 r0(state) r7(sp) r9(mul1) r10(ld3)
Assignee | ||
Comment 4•15 years ago
|
||
Jacob, what can we do here? The overflow check can be pessimistic so we could check for (a | b) & 0xffff0000 != 0 for example.
Assignee | ||
Comment 5•15 years ago
|
||
This will fix the issue until we have some code in place in the ARM backend.
Assignee | ||
Updated•15 years ago
|
Attachment #405198 -
Flags: review?(dvander)
Assignee | ||
Comment 6•15 years ago
|
||
If we are considering vlad's arm firefox stuff part of the 1.9.2 cycle than this should block 1.9.2, but it doesn't have to block the upcoming x86 beta.
Flags: blocking1.9.2?
Assignee | ||
Updated•15 years ago
|
Priority: -- → P2
Updated•15 years ago
|
Attachment #405198 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 7•15 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/43ee7e151862
Whiteboard: fixed-in-tracemonkey
Reporter | ||
Updated•15 years ago
|
OS: Mac OS X → Linux
Hardware: x86 → ARM
Updated•15 years ago
|
Flags: blocking1.9.2? → blocking1.9.2+
Comment 8•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/43ee7e151862
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 9•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ba365fe2fecc
status1.9.2:
--- → final-fixed
Comment 10•11 years ago
|
||
Bug in removed tracer code, setting in-testsuite- flag.
Flags: in-testsuite-
Updated•10 years ago
|
tracking-fennec: ? → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•