LGTM

http://codereview.chromium.org/465028/diff/1/3
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/465028/diff/1/3#newcode5207
src/arm/codegen-arm.cc:5207: __ mov(r0, Operand(1));  // Set number of
arguments.
Why is number of arguments 1 here, not 2?

http://codereview.chromium.org/465028/diff/1/2
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/465028/diff/1/2#newcode7732
src/x64/codegen-x64.cc:7732: __ InvokeBuiltin(Builtins::ADD,
JUMP_FUNCTION);
Still calling it first?

http://codereview.chromium.org/465028/diff/1/2#newcode7739
src/x64/codegen-x64.cc:7739: is_smi = __ CheckSmi(rax);
Use masm->CheckSmi(rax) (or masm() or masm_ or whatever __ expands to
here). The __ macro for doing coverage testing doesn't allow returning a
value.

http://codereview.chromium.org/465028/diff/1/2#newcode7745
src/x64/codegen-x64.cc:7745: is_smi = __ CheckSmi(rdx);
Ditto on __.

http://codereview.chromium.org/465028/diff/1/2#newcode7760
src/x64/codegen-x64.cc:7760: is_smi = __ CheckSmi(rdx);
And again.

http://codereview.chromium.org/465028

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

Reply via email to