[v8-dev] Re: X64: Added jmp and call and nop(n) to X64 assembler.

2009-05-29 Thread William Hesse
What I am saying is that the int version should take any int, and mask off all but 3 bits. Then it is safer, and can take the register code directly when implementing the register version. On Fri, May 29, 2009 at 3:13 PM, wrote: > > http://codereview.chromium.org/115920/diff/1008/1011 > File sr

[v8-dev] Re: X64: Added jmp and call and nop(n) to X64 assembler.

2009-05-29 Thread lrn
http://codereview.chromium.org/115920/diff/1008/1011 File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/115920/diff/1008/1011#newcode275 Line 275: ASSERT_EQ(rm & 0x07, rm); I'd rather fail than silently correct errors. The caller should know that the argument is valid. http:/

[v8-dev] Re: X64: Added jmp and call and nop(n) to X64 assembler.

2009-05-29 Thread whesse
LGTM. Comments are optional changes. http://codereview.chromium.org/115920/diff/1008/1010 File src/x64/assembler-x64-inl.h (right): http://codereview.chromium.org/115920/diff/1008/1010#newcode77 Line 77: emit(0x48 | (rm_reg.code() >> 3)); There was never a good reason for putting these in -inl

[v8-dev] Re: X64: Added jmp and call and nop(n) to X64 assembler.

2009-05-29 Thread lrn
http://codereview.chromium.org/115920/diff/1/3 File src/x64/assembler-x64-inl.h (right): http://codereview.chromium.org/115920/diff/1/3#newcode76 Line 76: emit(0x48 | (reg.code() >> 3)); I agree that there should be only one implementation. I would prefer it to be the one taking a numeric argume

[v8-dev] Re: X64: Added jmp and call and nop(n) to X64 assembler.

2009-05-29 Thread kmillikin
Fast progress on the assembler. Some drive-by comments. http://codereview.chromium.org/115920/diff/1/4 File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/115920/diff/1/4#newcode600 Line 600: void Assembler::nop(int n) { It is unclear how you are implemenenting nops of variou

[v8-dev] Re: X64: Added jmp and call and nop(n) to X64 assembler.

2009-05-29 Thread whesse
http://codereview.chromium.org/115920/diff/1/2 File src/ia32/assembler-ia32.cc (right): http://codereview.chromium.org/115920/diff/1/2#newcode1491 Line 1491: emit_rex_64(adr); I don't think this belongs in the ia32 compiler. http://codereview.chromium.org/115920/diff/1/3 File src/x64/assembler-