On Tue, Dec 22, 2009 at 10:49 AM, <[email protected]> wrote: > 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.
Done. > 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. Next CL. > 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). Done. > 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. Done. -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
