Chromium Code Reviews
Help | Chromium Project | Sign in
(69)

Issue 10411016: Fix for issue 2132. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 9 months ago by Massi
Modified:
1 month ago
CC:
v8-dev
Visibility:
Public.

Description

Fix for issue 2132. BUG=2132 TEST=

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -2 lines) Patch
M src/hydrogen.cc View 1 chunk +2 lines, -1 line 1 comment Download
M src/hydrogen-instructions.h View 2 chunks +5 lines, -0 lines 0 comments Download
M src/hydrogen-instructions.cc View 2 chunks +20 lines, -1 line 1 comment Download
Trybot results:
Commit:

Messages

Total messages: 3 (0 generated)
Massi
Since you opened the issue, could you review this?
2 years, 9 months ago (2012-05-18 11:08:38 UTC) #1
Florian Schneider
another idea: How about splitting Canonicalize phase into two (e.g. CanonicalizeEarly and CanonicalizeLate)? There are ...
2 years, 9 months ago (2012-05-18 16:12:06 UTC) #2
Vyacheslav Egorov
2 years, 9 months ago (2012-05-18 17:39:05 UTC) #3
I think Florian's suggestion is the way to go.

https://chromiumcodereview.appspot.com/10411016/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.cc (right):

https://chromiumcodereview.appspot.com/10411016/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.cc:880:
right()->range()->set_can_be_minus_zero(false);
I don't think this is correct. Consider code like:

function mul(a, b) {
  var x = a * b;
  x | 0;
  return x;
}

your code will remove minus zero from the range of x but this is incorrect.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld dd99357-tainted