On Tue, Jul 14, 2009 at 11:09 AM, <[email protected]> wrote: > 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.
Hey Bill, If you take care of this CL that would be great! I only have single comment: Why is the treatment of arch=arm and arch=x64 different in test.py? Weird. Cheers, Kasper > 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 -~----------~----~----~----~------~----~------~--~---
