Last Comment Bug 760103 - IonMonkey: Math.floor/Math.round correctness bug
: IonMonkey: Math.floor/Math.round correctness bug
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: IonMonkey
  Show dependency treegraph
 
Reported: 2012-05-31 07:24 PDT by Jan de Mooij [:jandem]
Modified: 2012-06-01 02:39 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (4.53 KB, patch)
2012-05-31 09:43 PDT, Jan de Mooij [:jandem]
sstangl: review+
Details | Diff | Splinter Review

Description Jan de Mooij [:jandem] 2012-05-31 07:24:13 PDT
With IonMonkey there's one new test262 failure compared to mozilla-inbound.

Here's a reduced testcase:

function f(x) {
    print(Math.round(x), Math.floor(x + 0.5));
}
f(0.2);
f(0.2);
f(0.2);

On OS X, 32-bit, revision efd4c7fc0697 I get the following output:

0 0
0 1
0 1

With --no-ion:

0 0
0 0
0 0
Comment 1 Jan de Mooij [:jandem] 2012-05-31 07:54:50 PDT
Forgot to mention in comment 0, the testcase requires --ion-eager...
Comment 2 Jan de Mooij [:jandem] 2012-05-31 09:43:19 PDT
Created attachment 628781 [details] [diff] [review]
Fix

The problem is that LRound overwrites its input register and the LFloor instruction used the modified value. The patch uses the temp register instead of the input register to hold the result of input + 0.5.

I temporarily disabled the AssemblerX86Shared::HasSSE41() check to test both branches (will file a follow-up bug to add a flag to disable SSE X support).
Comment 3 Jan de Mooij [:jandem] 2012-06-01 02:39:50 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/16141b0a3d12

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