LGTM, with comments. I can make the changes, if you want. It looks like the test result changes for x64 were mainly made in an earlier changelist.
http://codereview.chromium.org/155346/diff/1/3 File src/x64/disasm-x64.cc (right): http://codereview.chromium.org/155346/diff/1/3#newcode509 Line 509: int DisassemblerX64::PrintImmediate(byte* data, OperandSize size) { I would call the argument immediate_size, since the operand size of the instruction can be larger than the size of the immediate data. http://codereview.chromium.org/155346/diff/1/3#newcode622 Line 622: OperandSize op_size = byte_size_immediate ? BYTE_SIZE : operand_size(); immediate_size, not op_size. http://codereview.chromium.org/155346/diff/1/3#newcode732 Line 732: if (imm8 > 0) { This seems wrong. Why aren't we testing the opcode to see if it encodes using cl as the shift amount? Won't this guarantee that a shift of 0 (or is that 64?) is disassembled wrong? We may not generate that, but it may as well be disassembled correctly. http://codereview.chromium.org/155346 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
