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

Issue 10411016: Fix for issue 2132.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 11 months ago by Massi
Modified:
1 year, 11 months ago
CC:
v8-dev_googlegroups.com
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) Lint Patch
M src/hydrogen.cc View 1 chunk +2 lines, -1 line 1 comment ? errors Download
M src/hydrogen-instructions.h View 2 chunks +5 lines, -0 lines 0 comments 0 errors Download
M src/hydrogen-instructions.cc View 2 chunks +20 lines, -1 line 1 comment ? errors Download
Trybot results: Sign in to try more bots
Commit:

Issue must be closed to Revert

Messages

Total messages: 3
Massi
Since you opened the issue, could you review this?
1 year, 11 months ago #1
Florian Schneider
another idea: How about splitting Canonicalize phase into two (e.g. CanonicalizeEarly and CanonicalizeLate)? There are ...
1 year, 11 months ago #2
Vyacheslav Egorov
1 year, 11 months ago #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 1275:d14800f88434