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

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

Description Jan de Mooij [:jandem] (PTO until July 31) 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));

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] (PTO until July 31) 2012-05-31 07:54:50 PDT
Forgot to mention in comment 0, the testcase requires --ion-eager...
Comment 2 Jan de Mooij [:jandem] (PTO until July 31) 2012-05-31 09:43:19 PDT
Created attachment 628781 [details] [diff] [review]

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] (PTO until July 31) 2012-06-01 02:39:50 PDT

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