http://codereview.chromium.org/5107002/diff/1/src/arm/stub-cache-arm.cc File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/5107002/diff/1/src/arm/stub-cache-arm.cc#newcode1084 src/arm/stub-cache-arm.cc:1084: __ push(receiver); // Receiver On 2010/11/17 14:10:32, antonm wrote:
nit: probably change R -> r as you remove a dot.
Reverted change in this line. http://codereview.chromium.org/5107002/diff/1/src/arm/stub-cache-arm.cc#newcode1085 src/arm/stub-cache-arm.cc:1085: __ mov(ip, Operand(Handle<AccessorInfo>(callback))); // callback data On 2010/11/17 14:10:32, antonm wrote:
why such a change, __ Push(reg, ip, name_reg) apparently should have
done the
trick, what am I missing
It would work, but generates more instructions. |Push| preferes arguments with decreasing codes. ip=r13, scratch3=r4, name_reg=r0|r2. So here are 2 decreasing subsequences. Actually I was wrong (thought it needs increasing codes) and now fixing. http://codereview.chromium.org/5107002/diff/1/src/arm/stub-cache-arm.cc#newcode1211 src/arm/stub-cache-arm.cc:1211: __ ldr(scratch3, On 2010/11/17 14:10:32, antonm wrote:
why scratch3? it looks like the diff might be made easier to read by
something
like:
__ Push(receiver, holder_reg); __ ldr(....) __ Push(.......)
Done. http://codereview.chromium.org/5107002/diff/1/src/builtins.cc File src/builtins.cc (left): http://codereview.chromium.org/5107002/diff/1/src/builtins.cc#oldcode1102 src/builtins.cc:1102: custom.end(), On 2010/11/17 14:10:32, antonm wrote:
do we use CustomArguments? ::end()?
We do. http://codereview.chromium.org/5107002/diff/1/src/builtins.cc File src/builtins.cc (right): http://codereview.chromium.org/5107002/diff/1/src/builtins.cc#newcode1091 src/builtins.cc:1091: Handle<JSFunction> function = args.at<JSFunction>(args_length); On 2010/11/17 14:10:32, antonm wrote:
maybe move this check after new_args construction and use
new_args.Callee() and
new_args.This()?
new_args.Callee() is v8::Handle. Do you think it worth to convert it back and forth? http://codereview.chromium.org/5107002/diff/1/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/5107002/diff/1/test/cctest/test-api.cc#newcode7019 test/cctest/test-api.cc:7019: CHECK_EQ(args.Data(), v8_str("method_data")); On 2010/11/17 14:10:32, antonm wrote:
nit: indent
It was for debugging. Reverted. http://codereview.chromium.org/5107002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
