LGTM.

http://codereview.chromium.org/5714001/diff/9001/src/ia32/assembler-ia32.h
File src/ia32/assembler-ia32.h (right):

http://codereview.chromium.org/5714001/diff/9001/src/ia32/assembler-ia32.h#newcode576
src/ia32/assembler-ia32.h:576: static const byte kJmpShortOpcode = 0xeb;
I think we tend to use capital letters in hexadecimal constants in the
assembler.

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc#newcode1597
src/ia32/full-codegen-ia32.cc:1597: __ CallStub(&stub);
On 2010/12/09 15:59:01, Vitaly wrote:
Consider adding a function like EmitCallIC that takes both a stub to
call and an
optional patch site. Patch site should be used if it is bound,
otherwise nop is
emitted.

I'm not sure this is a good idea, since the existing call to CallStub is
at a nice low level, and all the other JumpSitePatch relevant stuff is
right here in the same code.

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc#newcode3767
src/ia32/full-codegen-ia32.cc:3767: __ jmp(&done);
Too bad that you have to put an extra jump here, to make it work.

http://codereview.chromium.org/5714001/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to