Hi Jakob,
Thanks for the look thus far. I uploaded a patch that responds to comments,
then
an arm/arm64 patch for ease of reading.
Yes, if I have some trouble I'll divide the CL up by platform. Until then
blithe confidence...:)
Thanks again,
--Michael
https://codereview.chromium.org/338963003/diff/30001/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):
https://codereview.chromium.org/338963003/diff/30001/src/ia32/builtins-ia32.cc#newcode1016
src/ia32/builtins-ia32.cc:1016: // Update the index on the stack and in
register eax.
On 2014/06/26 16:17:10, Jakob wrote:
nit: s/eax/|key|/
Done.
https://codereview.chromium.org/338963003/diff/30001/src/ia32/builtins-ia32.cc#newcode1028
src/ia32/builtins-ia32.cc:1028: ASSERT(!key.is(eax));
On 2014/06/26 16:17:10, Jakob wrote:
Here, too, consider using Move() instead.
Done.
https://codereview.chromium.org/338963003/diff/30001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):
https://codereview.chromium.org/338963003/diff/30001/src/x64/builtins-x64.cc#newcode1100
src/x64/builtins-x64.cc:1100: ASSERT(!key.is(rax));
On 2014/06/26 16:17:10, Jakob wrote:
why does this matter? AFAICS violating the ASSERT wouldn't hurt, so we
can just
skip it.
Done.
https://codereview.chromium.org/338963003/diff/30001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):
https://codereview.chromium.org/338963003/diff/30001/src/x64/full-codegen-x64.cc#newcode2069
src/x64/full-codegen-x64.cc:2069: ASSERT(!load_receiver.is(rax));
On 2014/06/26 16:17:10, Jakob wrote:
suggestion: in all the cases where you're ASSERTing that we're not
moving
unnecessarily, you could simply use Move() instead of ASSERT+movp().
Move()
does a src != dst check.
Awesome, didn't realize that, thx.
https://codereview.chromium.org/338963003/diff/30001/src/x64/ic-x64.cc
File src/x64/ic-x64.cc (right):
https://codereview.chromium.org/338963003/diff/30001/src/x64/ic-x64.cc#newcode368
src/x64/ic-x64.cc:368: // receiver: receiver
On 2014/06/26 16:17:10, Jakob wrote:
Yay for high-entropy comments! :-P
AFAICS, the |receiver| register always contains the receiver, and the
|key|
register always contains the key (it is only overwritten with the
"new" key in
case of heapnumber->smi conversion). So you could drop the comments
about those
two.
Will do, thanks.
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.
