Thanks Toon, here are the refactorings we discussed,
--Michael


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

https://codereview.chromium.org/172523002/diff/280001/src/ia32/code-stubs-ia32.cc#newcode2456
src/ia32/code-stubs-ia32.cc:2456: if (attributes ==
CallIC::NOT_STRICT_OR_NATIVE) {
On 2014/03/24 10:25:44, Toon Verwaest wrote:
CallIC::SLOPPY

Seems like in this case you don't need the checks above.

Good catch, thanks. Done.

https://codereview.chromium.org/172523002/diff/280001/src/ia32/code-stubs-ia32.cc#newcode2469
src/ia32/code-stubs-ia32.cc:2469: if (argument_check ==
CallIC::ARGUMENTS_MATCH) {
On 2014/03/24 10:25:44, Toon Verwaest wrote:
ARGUMENTS_MUST_MATCH, to distinguish from the other case,
ARGUMENTS_COUNT_UNKNOWN

Done.

https://codereview.chromium.org/172523002/diff/280001/src/ia32/code-stubs-ia32.cc#newcode2527
src/ia32/code-stubs-ia32.cc:2527: __ mov(ecx, FieldOperand(ebx, edx,
times_half_pointer_size,
On 2014/03/24 10:25:44, Toon Verwaest wrote:
__ cmp(edi, Field...

Done.

https://codereview.chromium.org/172523002/diff/280001/src/ia32/code-stubs-ia32.cc#newcode2532
src/ia32/code-stubs-ia32.cc:2532: if (stub_type() !=
CallIC::MONOMORPHIC) {
On 2014/03/24 10:25:44, Toon Verwaest wrote:
stub_type() == CallIC::DEFAULT_MONOMORPHIC

Done.

https://codereview.chromium.org/172523002/diff/280001/src/ia32/code-stubs-ia32.cc#newcode2535
src/ia32/code-stubs-ia32.cc:2535: CallIC::ARGUMENTS_MATCH,
On 2014/03/24 10:25:44, Toon Verwaest wrote:
Can we merge this with the non-default case by just setting up
argument_check()
and function_attributes() correctly for the default case?

Yes indeed, thanks!

https://codereview.chromium.org/172523002/diff/280001/src/ic.cc
File src/ic.cc (right):

https://codereview.chromium.org/172523002/diff/280001/src/ic.cc#newcode1836
src/ic.cc:1836: return;
On 2014/03/24 10:25:44, Toon Verwaest wrote:
Patch in the generic case, which doesn't record anymore, but also
doesn't check.

I fixed this to patch to megamorphic in this case, but I don't have a
real GENERIC stub, I only have:

DEFAULT_MONOMORPHIC - maintains type cell, embeds a popular monomorphic
case, handles everything.

MONOMORPHIC - maintains type cell, but customized to the function.

I think it's okay to plug in DEFAULT_MONOMORPHIC. It does mean we keep
checking against the MEGAMORPHIC sentinel, but this doesn't need to be a
faster path anyway. The old code did the same thing.

https://codereview.chromium.org/172523002/

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