LGTM with comments.


http://codereview.chromium.org/975001/diff/1/4
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/975001/diff/1/4#newcode6999
src/ia32/codegen-ia32.cc:6999: if (left.is_register() &&
left.reg().is(edi)) {
You could add the case

right.reg().is(eax) && left.is_register()

and do an xchg instruction.

http://codereview.chromium.org/975001/diff/1/4#newcode7010
src/ia32/codegen-ia32.cc:7010: // TODO(whesse): fix it
May comment why there is no need to fix it now (e.g. does not affect
generated code?) - or remove todo comment.

http://codereview.chromium.org/975001/diff/1/11
File src/ia32/register-allocator-ia32.cc (right):

http://codereview.chromium.org/975001/diff/1/11#newcode67
src/ia32/register-allocator-ia32.cc:67: // Constant is not a number.
This was not predicted by AST analysis.
Fix indentation.

http://codereview.chromium.org/975001/diff/1/6
File src/ia32/virtual-frame-ia32.cc (right):

http://codereview.chromium.org/975001/diff/1/6#newcode822
src/ia32/virtual-frame-ia32.cc:822: __ cmp(FieldOperand(fresh_reg,
HeapObject::kMapOffset),
Wrap the map check for heap number in a check for number info (and
remove the todo comment):

if (!original.number_info().IsNumber()) {...}

http://codereview.chromium.org/975001

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to