Last Comment Bug 736141 - IonMonkey: x + 0 equals x only when x can't be -0
: IonMonkey: x + 0 equals x only when x can't be -0
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Hannes Verschore [:h4writer]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-15 09:48 PDT by Hannes Verschore [:h4writer]
Modified: 2012-03-20 04:04 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for both issues (1021 bytes, patch)
2012-03-15 10:14 PDT, Hannes Verschore [:h4writer]
dvander: review+
Details | Diff | Splinter Review
Patch for both issues (1.70 KB, patch)
2012-03-16 03:41 PDT, Hannes Verschore [:h4writer]
hv1989: review+
Details | Diff | Splinter Review

Description Hannes Verschore [:h4writer] 2012-03-15 09:48:15 PDT
During GVN we fold values. E.g. in the addition we fold the addition to x + 0 to x.
This isn't true if x equals -0. 

testcase:

function test(i) {
    return i * 0 + 0;
}

// Compile double variant
for(var i=0; i<100; i++){
    test(-i);
}

var x = test(-100);
// Prints if x == -0
print((x===0 && (1/x)===-Infinity));

prints true in Ionmonkey, false in interpreter.

Reason:
-0 + 0 = +0 // positive zero
but we remove the addition (as 0 is the identity function of addition)

So the test function becomes:

function test(i){
   return i * 0
}

Which gives negative zero when i < 0
Comment 1 Hannes Verschore [:h4writer] 2012-03-15 09:53:19 PDT
Another fault in foldsto:

function test(i) {
    return 0 - i;
}

is folded to:

function test(i) {
    return i; // NOTICE THE LACK OF SIGN
}
Comment 2 Hannes Verschore [:h4writer] 2012-03-15 10:14:17 PDT
Created attachment 606278 [details] [diff] [review]
Patch for both issues

This fixes both issues. I'll add both testcases and commit msg after review.
Comment 3 Robert Longson 2012-03-15 10:35:44 PDT
s/substraction/subtraction/
Comment 4 David Anderson [:dvander] 2012-03-15 17:52:28 PDT
Comment on attachment 606278 [details] [diff] [review]
Patch for both issues

Review of attachment 606278 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch!
Comment 5 Hannes Verschore [:h4writer] 2012-03-16 03:41:09 PDT
Created attachment 606517 [details] [diff] [review]
Patch for both issues

Robert's nit addressed, together with commit description and author. Ready for checkin.
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-03-19 16:05:40 PDT
Jan, can you check this in?
Comment 7 Jan de Mooij [:jandem] (PTO until July 31) 2012-03-20 04:04:26 PDT
(In reply to Ryan VanderMeulen from comment #6)
> Jan, can you check this in?

Sure: http://hg.mozilla.org/projects/ionmonkey/rev/3a4607b336a8

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