Addressed comments. Thanks for all the feedback!

https://codereview.chromium.org/1237813002/diff/20001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/arm/code-stubs-arm.cc#newcode2622
src/arm/code-stubs-arm.cc:2622: __ push(r4);
On 2015/07/14 16:57:35, jbramley wrote:
On 2015/07/14 15:02:47, Michael Starzinger wrote:
> Potentially applies to all architectures: Note that I realize that
on some
> architectures we could probably avoid this push and the pop a few
lines below,
> by massaging GenerateRecordCallTarget appropriately so that it
preserves the
> register in question. However I would like to do this in a follow-up
CL if
> possible. But I am of course happy to leave a TODO for all
architectures where
> this might apply, just let me know.

A TODO is probably a good idea, but it's up to you. On ARM64,
GenerateRecordCallTarget takes a list of scratch registers, and that
approach
works quite well.

Done. Added a TODO here.

https://codereview.chromium.org/1237813002/diff/20001/src/arm64/code-stubs-arm64.cc
File src/arm64/code-stubs-arm64.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/arm64/code-stubs-arm64.cc#newcode3004
src/arm64/code-stubs-arm64.cc:3004: GenerateRecordCallTarget(masm, x0,
function, x2, x3, x4, x5, x11);
On 2015/07/14 16:57:35, jbramley wrote:
ARM64's GenerateRecordCallTarget allows the scratch registers to be
specified,
so it should be possible to just specify something other than x4 and
skip the
push/pop sequence.

Acknowledged. I initially tried that, but CallStubInRecordCallTarget
still clobbers x4. But I left a TODO here.

https://codereview.chromium.org/1237813002/diff/20001/src/arm64/full-codegen-arm64.cc
File src/arm64/full-codegen-arm64.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/arm64/full-codegen-arm64.cc#newcode3093
src/arm64/full-codegen-arm64.cc:3093: __ mov(x4, result_register());
On 2015/07/14 16:57:35, jbramley wrote:
We prefer the MacroAssembler's "__ Mov(...)" in ARM64, for
consistency, even
though it doesn't make any difference in this particular case. (The
lower-case
Assembler versions are usually reserved for critical sections in
InstructionAccurateScopes.)

Done. One of these days I am gonna remember that, I promise. :)

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/builtins-mips64.cc
File src/mips64/builtins-mips64.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/builtins-mips64.cc#newcode1511
src/mips64/builtins-mips64.cc:1511: __ ld(t0, MemOperand(fp,
kNewTargetOffset));
On 2015/07/14 16:58:17, paul.l... wrote:
t0 -> a4

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc
File src/mips64/code-stubs-mips64.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc#newcode2784
src/mips64/code-stubs-mips64.cc:2784: // t0 : original constructor (for
IsSuperConstructorCall)
On 2015/07/14 16:58:17, paul.l... wrote:
Please change t0 -> a4

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc#newcode2790
src/mips64/code-stubs-mips64.cc:2790: __ Branch(&slow, ne, a4,
Operand(JS_FUNCTION_TYPE));
On 2015/07/14 16:58:18, paul.l... wrote:
Please change these 3 uses of a4 -> a5

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc#newcode2794
src/mips64/code-stubs-mips64.cc:2794: __ push(t0);
On 2015/07/14 16:58:17, paul.l... wrote:
Please use a4 here and in pop() below.

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc#newcode2824
src/mips64/code-stubs-mips64.cc:2824: __ mov(a3, t0);
On 2015/07/14 16:58:18, paul.l... wrote:
t0 -> a4

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc#newcode2839
src/mips64/code-stubs-mips64.cc:2839: // a4: object type
On 2015/07/14 16:58:18, paul.l... wrote:
a4 -> a5 for object type

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/code-stubs-mips64.cc#newcode2842
src/mips64/code-stubs-mips64.cc:2842: __ Branch(&non_function_call, ne,
a4, Operand(JS_FUNCTION_PROXY_TYPE));
On 2015/07/14 16:58:18, paul.l... wrote:
a4 -> a5

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/full-codegen-mips64.cc
File src/mips64/full-codegen-mips64.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/full-codegen-mips64.cc#newcode3388
src/mips64/full-codegen-mips64.cc:3388: __ mov(t0, result_register());
On 2015/07/14 16:58:18, paul.l... wrote:
t0 -> a4 here, and in comment above.

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/full-codegen-mips64.cc#newcode4348
src/mips64/full-codegen-mips64.cc:4348: __ ld(t0, MemOperand(sp, 1 *
kPointerSize));
On 2015/07/14 16:58:18, paul.l... wrote:
t0 -> a4 here and in comment.

Done.

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/interface-descriptors-mips64.cc
File src/mips64/interface-descriptors-mips64.cc (right):

https://codereview.chromium.org/1237813002/diff/20001/src/mips64/interface-descriptors-mips64.cc#newcode176
src/mips64/interface-descriptors-mips64.cc:176: Register registers[] =
{a0, a1, t0, a2};
On 2015/07/14 16:58:18, paul.l... wrote:
t0 ->a4 here and in comment above (line 173)

Done.

https://codereview.chromium.org/1237813002/

--
--
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.

Reply via email to