Closed Bug 521152 Opened 15 years ago Closed 15 years ago

TM: factorial computed incorrectly on ARM

Categories

(Core :: JavaScript Engine, defect, P2)

ARM
Linux
defect

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)

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: general → gal
tracking-fennec: --- → ?
Great testcase. Should be an easy fix. Trying to reproduce.
Looks like ARM doesn't set the overflow flag on multiply. That is very sad.
  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)
Jacob, what can we do here? The overflow check can be pessimistic so we could check for (a | b) & 0xffff0000 != 0 for example.
This will fix the issue until we have some code in place in the ARM backend.
Attachment #405198 - Flags: review?(dvander)
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?
Priority: -- → P2
Attachment #405198 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/43ee7e151862
Whiteboard: fixed-in-tracemonkey
Depends on: 521161
OS: Mac OS X → Linux
Hardware: x86 → ARM
Flags: blocking1.9.2? → blocking1.9.2+
http://hg.mozilla.org/mozilla-central/rev/43ee7e151862
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Bug in removed tracer code, setting in-testsuite- flag.
Flags: in-testsuite-
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: