LGTM with a bunch of nits.
Great work!
https://codereview.chromium.org/338963003/diff/110001/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):
https://codereview.chromium.org/338963003/diff/110001/src/arm/full-codegen-arm.cc#newcode2631
src/arm/full-codegen-arm.cc:2631: // Move the key into the right
register for the keyed load IC.
nit: unnecessary comment (doesn't say anything that couldn't be seen in
the code)
https://codereview.chromium.org/338963003/diff/110001/src/arm/full-codegen-arm.cc#newcode4228
src/arm/full-codegen-arm.cc:4228: // Put the object both on the stack
and in the accumulator.
nit: s/accumulator/register/
https://codereview.chromium.org/338963003/diff/110001/src/arm/ic-arm.cc
File src/arm/ic-arm.cc (right):
https://codereview.chromium.org/338963003/diff/110001/src/arm/ic-arm.cc#newcode317
src/arm/ic-arm.cc:317: // -- r1 : receiver
Why doesn't this get the same treatment as e.g.
KeyedLoadIC::GenerateSloppyArguments below?
https://codereview.chromium.org/338963003/diff/110001/src/arm/ic-arm.cc#newcode475
src/arm/ic-arm.cc:475: // The return address is on the stack.
actually, it's in |lr|
https://codereview.chromium.org/338963003/diff/110001/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):
https://codereview.chromium.org/338963003/diff/110001/src/arm/stub-cache-arm.cc#newcode1478
src/arm/stub-cache-arm.cc:1478: // -- lr : return address
another comment that we an trim to just this line.
https://codereview.chromium.org/338963003/diff/110001/src/arm/stub-cache-arm.cc#newcode1501
src/arm/stub-cache-arm.cc:1501: // -- r0 : key
outdated. Just drop the entire comment.
https://codereview.chromium.org/338963003/diff/110001/src/arm/stub-cache-arm.cc#newcode1511
src/arm/stub-cache-arm.cc:1511: // -- r0 : key
again
https://codereview.chromium.org/338963003/diff/110001/src/arm64/full-codegen-arm64.cc
File src/arm64/full-codegen-arm64.cc (right):
https://codereview.chromium.org/338963003/diff/110001/src/arm64/full-codegen-arm64.cc#newcode2330
src/arm64/full-codegen-arm64.cc:2330: // Move the key into the right
register for the keyed load IC call.
don't need this comment.
https://codereview.chromium.org/338963003/diff/110001/src/arm64/ic-arm64.cc
File src/arm64/ic-arm64.cc (right):
https://codereview.chromium.org/338963003/diff/110001/src/arm64/ic-arm64.cc#newcode412
src/arm64/ic-arm64.cc:412: // -- lr : return address
another comment that can be trimmed to "The return address is in lr.",
at least if you use named registers below, possibly instead of the
ASSERTs.
https://codereview.chromium.org/338963003/diff/110001/src/arm64/ic-arm64.cc#newcode496
src/arm64/ic-arm64.cc:496: __ Ldr(x0, unmapped_location);
I'd s/x0/result/ here (and in the next line)
https://codereview.chromium.org/338963003/diff/110001/src/arm64/ic-arm64.cc#newcode762
src/arm64/ic-arm64.cc:762: // Slow case
nit: trailing full stop please.
https://codereview.chromium.org/338963003/diff/110001/src/arm64/ic-arm64.cc#newcode780
src/arm64/ic-arm64.cc:780: void
KeyedLoadIC::GenerateString(MacroAssembler* masm) {
Wohoo! Register refactoring nirvana -- nothing to do here :-)
https://codereview.chromium.org/338963003/diff/110001/src/arm64/stub-cache-arm64.cc
File src/arm64/stub-cache-arm64.cc (right):
https://codereview.chromium.org/338963003/diff/110001/src/arm64/stub-cache-arm64.cc#newcode1453
src/arm64/stub-cache-arm64.cc:1453: // -- lr : return address
one more comment to trim.
https://codereview.chromium.org/338963003/
--
--
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.
