All files except the two new x64 ones reviewed. Will send their review later.
http://codereview.chromium.org/165443/diff/42/1006 File src/jsregexp.cc (right): http://codereview.chromium.org/165443/diff/42/1006#newcode469 Line 469: // If result is RETRY, the string have changed representation, and we Grammar. "string have" http://codereview.chromium.org/165443/diff/42/1014 File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/165443/diff/42/1014#newcode449 Line 449: void Assembler::arithmetic_op_16(byte opcode, Register dst, Register src) { These, and the other ones, should be reg, rm_reg. http://codereview.chromium.org/165443/diff/42/1014#newcode814 Line 814: emit(0x3c); Shouldn't imm8.value_ be emitted? http://codereview.chromium.org/165443/diff/42/1015 File src/x64/assembler-x64.h (right): http://codereview.chromium.org/165443/diff/42/1015#newcode288 Line 288: private: This looks wrongly formatted. Shouldn't the blank line come before "private:" http://codereview.chromium.org/165443/diff/42/1015#newcode543 Line 543: arithmetic_op_32(0x02, src, dst); I think you need 0x01 here, not 0x02. http://codereview.chromium.org/165443/diff/42/1015#newcode588 Line 588: arithmetic_op(0x38, dst, src); I think this is backwards. 0x38 is cmp r/m, reg. The second argument of arithmetic op is reg_rm, I think. Should you change all reg, reg operations to avoid the SIB byte for sp/r12, or was it just important for addl? http://codereview.chromium.org/165443/diff/42/1015#newcode600 Line 600: immediate_arithmetic_op_8(0x7, dst, src); Should this test if dst is al, and call cmpb_al? If so, why is cmpb_al publicly exposed? http://codereview.chromium.org/165443/diff/42/1015#newcode1193 Line 1193: void arithmetic_op_32(byte opcode, Register dst, Register src); All of these should say reg, rm_reg, not dst, src, to agree with their parameters in the .cc file. The two 64-bit ones should be together. http://codereview.chromium.org/165443 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
