This is getting there.
http://codereview.chromium.org/2280007/diff/20001/21011 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/2280007/diff/20001/21011#newcode1116 src/ia32/ic-ia32.cc:1116: CallIC::GenerateNormal(masm, argc, call_mode, false); Normal here means normalized properties, this is not what we want to generate here. GenerateNormal that does not probe the dictionary does not make sense. If you want to reuse that code, please extract a static helper function CheckFunctionAndInvoke or something like that and use it here and in GenerateNormalHelper. Also, GenerateNormal will check that a lot of stuff about the receiver that the KeyedLoadIC should have already checked... http://codereview.chromium.org/2280007/diff/20001/21012 File src/ic.cc (right): http://codereview.chromium.org/2280007/diff/20001/21012#newcode155 src/ic.cc:155: // For keyed load/store, the most likely cause of cache failure is Update comment. http://codereview.chromium.org/2280007/diff/20001/21012#newcode160 src/ic.cc:160: target->ic_call_mode() == KEYED_CALL) { KEYED_CALL ICs are a different kind of IC. Why don't you introduce a separate kind instead of having a separate call_mode flag? The keyed call IC stubs needs to be treated like a completely different kind, so I don't see a reason to treat them in a special way when it comes to the kind encoding. http://codereview.chromium.org/2280007/diff/20001/21012#newcode492 src/ic.cc:492: if (object->IsString() || object->IsNumber() || object->IsBoolean()) { Check for undefined and null to generate error messages that match that of other calls and to not generate code when we know it will miss? http://codereview.chromium.org/2280007/diff/20001/21012#newcode1384 src/ic.cc:1384: CallIC ic(KEYED_CALL); It would be nicer to have a KeyedCallIC instance here. I find it confusing that we have these separate stubs that are of a completely separate kind but we do not have the KeyedCallIC class that I would expect. http://codereview.chromium.org/2280007/diff/20001/21012#newcode1389 src/ic.cc:1389: // The first time the inline cache is updated may be the first time the Maybe extract the rest of this function into a static helper called something like EnsureCompiledIfFunction and use it from CallIC_Miss and KeyedCallIC_Miss? http://codereview.chromium.org/2280007/diff/20001/21013 File src/ic.h (right): http://codereview.chromium.org/2280007/diff/20001/21013#newcode188 src/ic.h:188: class CallIC: public IC { I would prefer to have a KeyedCallIC class instead of putting in a flag in the CallIC. KeyedCallIC should be treated like a completely different kind of IC. You should still be able to share almost everything by using static helper functions? http://codereview.chromium.org/2280007/diff/20001/21018 File src/objects-inl.h (right): http://codereview.chromium.org/2280007/diff/20001/21018#newcode2240 src/objects-inl.h:2240: CallModeFlag Code::ic_call_mode() { I don't like this much. We should just have one more IC kind instead. That would avoid the rest of the changes to this file as well. http://codereview.chromium.org/2280007/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
