I have completely removed CallModeFlag and introduced KeyedCallIC. Reusing generating functions in ic-ia32.cc in a more transparent manner. Will add a test soon.
http://codereview.chromium.org/2280007/diff/20001/21005 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/2280007/diff/20001/21005#newcode5953 src/ia32/codegen-ia32.cc:5953: Load(property->obj()); On 2010/05/31 13:05:50, antonm wrote:
nit: maybe unify with lines 5917--5936 (add a comment, blank line)
Done. 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); Stopped reusing GenerateNormal, inlined some code instead. Will refactor for more inlining in the next CL. On 2010/05/31 13:14:08, Mads Ager wrote:
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/21011#newcode1131 src/ia32/ic-ia32.cc:1131: // -- edi : function to call (if lookup=true) Removed. On 2010/05/31 13:05:50, antonm wrote:
if lookup != true?
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 On 2010/05/31 13:14:08, Mads Ager wrote:
Update comment.
Done. http://codereview.chromium.org/2280007/diff/20001/21012#newcode160 src/ic.cc:160: target->ic_call_mode() == KEYED_CALL) { checking the kind now, call mode went away On 2010/05/31 13:05:50, antonm wrote:
shouldn't you check CALL_IC kind before?
http://codereview.chromium.org/2280007/diff/20001/21012#newcode492 src/ic.cc:492: if (object->IsString() || object->IsNumber() || object->IsBoolean()) { On 2010/05/31 13:14:08, Mads Ager wrote:
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?
Done. http://codereview.chromium.org/2280007/diff/20001/21012#newcode492 src/ic.cc:492: if (object->IsString() || object->IsNumber() || object->IsBoolean()) { On 2010/05/31 13:14:08, Mads Ager wrote:
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?
Done. http://codereview.chromium.org/2280007/diff/20001/21012#newcode1384 src/ic.cc:1384: CallIC ic(KEYED_CALL); On 2010/05/31 13:14:08, Mads Ager wrote:
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.
Done. 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 On 2010/05/31 13:14:08, Mads Ager wrote:
Maybe extract the rest of this function into a static helper called
something
like EnsureCompiledIfFunction and use it from CallIC_Miss and
KeyedCallIC_Miss? Extracted the actual compilation and left the first check inline (otherwise the function gets a bit slower on tests). 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 { Created a separate class KeyedCallIC. It is not that easy to extract UpdateCaches into a static (as it calls IC::set_target). On 2010/05/31 13:14:08, Mads Ager wrote:
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() { On 2010/05/31 13:14:08, Mads Ager wrote:
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.
Done. But I needed to change ComputeFlag for a different reason (to distinguish miss stubs for CALL_IC and KEYED_CALL_IC) http://codereview.chromium.org/2280007/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
