LGTM

http://codereview.chromium.org/503079/diff/1/4
File bleeding_edge/src/codegen.h (right):

http://codereview.chromium.org/503079/diff/1/4#newcode311
bleeding_edge/src/codegen.h:311: return OpField::encode(op_) |
OverwriteField::encode(overwrite_);
Bingo!

http://codereview.chromium.org/503079/diff/1/4#newcode319
bleeding_edge/src/codegen.h:319: : "GenericUnaryOpStub_Alloc";
This is annoying in profiles.  Please make it produce a more informative
name.  See the GenericBinaryOpstub for one way to do it.

http://codereview.chromium.org/503079/diff/1/3
File bleeding_edge/src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/503079/diff/1/3#newcode7702
bleeding_edge/src/ia32/codegen-ia32.cc:7702: if (op_ == Token::SUB) {
If I were doing this I would divide this function into two.  It's too
big.

http://codereview.chromium.org/503079/diff/1/3#newcode7781
bleeding_edge/src/ia32/codegen-ia32.cc:7781: __ cvtsi2sd(xmm0,
Operand(ecx));
This is good, so we should do it for SHR too (search for fstp_d).

http://codereview.chromium.org/503079/diff/1/2
File bleeding_edge/test/mjsunit/bit-not.js (right):

http://codereview.chromium.org/503079/diff/1/2#newcode45
bleeding_edge/test/mjsunit/bit-not.js:45: testBitNot(100);
We should have a loop so we can test the case where the allocation
fails.

http://codereview.chromium.org/503079

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

Reply via email to