LGTM

http://codereview.chromium.org/5055004/diff/1/src/ia32/macro-assembler-ia32.cc
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/5055004/diff/1/src/ia32/macro-assembler-ia32.cc#newcode429
src/ia32/macro-assembler-ia32.cc:429: // Restore current context from
top and clear it in debug mode.
I'd suggest (but not insisting) to factor out common exit frame
management.  Should be simple if push(ecx) is moved out of common code.

http://codereview.chromium.org/5055004/diff/1/src/ia32/macro-assembler-ia32.h
File src/ia32/macro-assembler-ia32.h (right):

http://codereview.chromium.org/5055004/diff/1/src/ia32/macro-assembler-ia32.h#newcode511
src/ia32/macro-assembler-ia32.h:511: // Returning removes stack_space *
kPointerSize (GCed).
I'd use 'on return' instead of 'returning'

http://codereview.chromium.org/5055004/diff/1/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/5055004/diff/1/src/ia32/stub-cache-ia32.cc#newcode502
src/ia32/stub-cache-ia32.cc:502: __ PrepareCallApiFunction(kApiArgc +
kApiStackSpace, ebx);
nice that the bug was spotted.  but I am slightly uneasy that our tests
didn't catch it.  can you add a test that demonstrates the presence of
the bug?

http://codereview.chromium.org/5055004/diff/1/src/ia32/stub-cache-ia32.cc#newcode2173
src/ia32/stub-cache-ia32.cc:2173: // before call any runtime function.
before call <to> any runtime

http://codereview.chromium.org/5055004/

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

Reply via email to