Right on, thanks! Will submit...
--Michael

https://codereview.chromium.org/352583002/diff/80001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/352583002/diff/80001/src/code-stubs.cc#newcode44
src/code-stubs.cc:44: // Also the register parameter representations
array if one is specified.
On 2014/06/25 10:31:56, Jakob wrote:
nit: this sentence no verb.

Done.

https://codereview.chromium.org/352583002/diff/80001/src/code-stubs.cc#newcode566
src/code-stubs.cc:566: ASSERT_EQ(KeyedLoadIC::kRegisterArgumentCount,
2);
On 2014/06/25 10:31:56, Jakob wrote:
nit: not sure I see the value of this ASSERT... but if we go on
refactoring this
code, it'll most likely be temporary anyway, so I don't care much.
Can you make it a STATIC_ASSERT though?

Done (static assert). The reason for it is that this system is
vulnerable to a change in the number of arguments for KeyedLoadIC. As
we've discussed there are ideas going forward to possibly avoid creating
an array of registers here, but make the KeyedLoadIC more directly
responsible.

https://codereview.chromium.org/352583002/diff/80001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/352583002/diff/80001/src/ia32/code-stubs-ia32.cc#newcode98
src/ia32/code-stubs-ia32.cc:98: Register registers[] = { edx };
On 2014/06/25 10:31:56, Jakob wrote:
I think this could use LoadIC::ReceiverRegister() already... but since
we have
to do more cleanup anyway, feel free to punt.

Good catch, it's in the next CL!

https://codereview.chromium.org/352583002/

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