Addressed comments. Thanks!
https://codereview.chromium.org/1227163011/diff/40001/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):
https://codereview.chromium.org/1227163011/diff/40001/src/arm/builtins-arm.cc#newcode357
src/arm/builtins-arm.cc:357: // Original constructor and function are
different.
On 2015/07/13 12:46:52, jbramley wrote:
This comment is now out of place. Consider something like this: "Fall
back to
runtime if the original constructor and function are different."
Done. This applies to all architectures. Now that you mention it, I
agree that the formulation might be misleading. Fixed for all of them.
https://codereview.chromium.org/1227163011/diff/40001/src/arm/builtins-arm.cc#newcode497
src/arm/builtins-arm.cc:497: __ ldr(r3, MemOperand(sp, 0 *
kPointerSize));
On 2015/07/13 12:46:52, jbramley wrote:
`MemOperand(sp)` would do here.
Acknowledged. Hmm, I thought having this explicit would increase
readability. Also we already do this in Generate_NotifyDeoptimizedHelper
and a couple of asserts. But I don't feel strongly about this.
https://codereview.chromium.org/1227163011/diff/40001/src/arm64/builtins-arm64.cc
File src/arm64/builtins-arm64.cc (right):
https://codereview.chromium.org/1227163011/diff/40001/src/arm64/builtins-arm64.cc#newcode354
src/arm64/builtins-arm64.cc:354: // Original constructor and function
are different.
On 2015/07/13 12:46:52, jbramley wrote:
Same comment as for ARM.
Done. Same on all architectures.
https://codereview.chromium.org/1227163011/diff/40001/src/arm64/builtins-arm64.cc#newcode505
src/arm64/builtins-arm64.cc:505: __ bind(&rt_call_reload_new_target);
On 2015/07/13 12:46:52, jbramley wrote:
For ARM64, we use the MacroAssembler's "Bind" for consistency, unless
in a
critical section where we're using the Assembler directly.
Done.
https://codereview.chromium.org/1227163011/diff/40001/src/arm64/builtins-arm64.cc#newcode519
src/arm64/builtins-arm64.cc:519: __ Push(original_constructor); //
argument 3: original constructor
On 2015/07/13 12:46:52, jbramley wrote:
For ARM64, it's preferable to merge pushes wherever possible. Each
call to Push
has a stack-maintenance overhead. The arguments to Push(...) are
ordered the
same as if they were listed separately. In this case:
__ Push(x4, constructor, original_constructor);
Done.
https://codereview.chromium.org/1227163011/diff/40001/src/arm64/builtins-arm64.cc#newcode525
src/arm64/builtins-arm64.cc:525: __ jmp(&count_incremented);
On 2015/07/13 12:46:53, jbramley wrote:
I think we'd normally use "__ B(&label)". ARM64 doesn't have a "jmp"
instruction
but it exists in the assembler for compatibility with other bits of
V8.
Done.
https://codereview.chromium.org/1227163011/diff/40001/src/arm64/builtins-arm64.cc#newcode528
src/arm64/builtins-arm64.cc:528: __ Push(original_constructor); //
argument 2: original constructor
On 2015/07/13 12:46:52, jbramley wrote:
Merge the pushes again.
Done.
https://codereview.chromium.org/1227163011/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
