Committed as change http://codereview.chromium.org/149608
Line 509: size left as is. Line 622: op_size changed to immediate_size Line 732: Suggested change made. n 2009/07/14 09:09:22, William Hesse 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. > 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 v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
