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. On 2010/11/16 14:32:26, antonm wrote:
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.
Done. 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). On 2010/11/16 14:32:26, antonm wrote:
I'd use 'on return' instead of 'returning'
Done. 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); On 2010/11/16 14:32:26, antonm wrote:
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? I don't see any bug. This line changed not to fix a bug but because signature of PrepareCallApiFunction had changed. It doesn't receive number of JS references anymore since now that references are removed in TryCallApiFunctionAndReturn. 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. On 2010/11/16 14:32:26, antonm wrote:
before call <to> any runtime
Done. http://codereview.chromium.org/5055004/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
