LGTM. The comments for the 32 bit version mostly apply to the 64 bit version too.
I don't see a performance bot run of the 64 bit version, which would have provided a nice verification that the optimization was working there. http://codereview.chromium.org/460142/diff/1/8 File src/heap.h (right): http://codereview.chromium.org/460142/diff/1/8#newcode1297 src/heap.h:1297: static const int kLength = 64; On a related note we should run a few performance bot runs with different values for this to see if it makes any difference. http://codereview.chromium.org/460142/diff/1/3 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/460142/diff/1/3#newcode53 src/ia32/ic-ia32.cc:53: Register name, bool check_dictionary) { This should be an enum with CHECK_DICTIONARY and DICTIONARY_CHECK_DONE http://codereview.chromium.org/460142/diff/1/3#newcode83 src/ia32/ic-ia32.cc:83: // Possible work-around for http://crbug.com/16276. Hmmm. Did this work? http://codereview.chromium.org/460142/diff/1/3#newcode307 src/ia32/ic-ia32.cc:307: __ test(ebx, Immediate(String::kIsArrayIndexMask)); Can't we do this with a single test instruction instead of loading into a register first? http://codereview.chromium.org/460142/diff/1/3#newcode312 src/ia32/ic-ia32.cc:312: __ test(ebx, Immediate(kIsSymbolMask)); And here? http://codereview.chromium.org/460142/diff/1/3#newcode322 src/ia32/ic-ia32.cc:322: // Load the map of the receiver, compute the keyed lookup cache hash Don't we need to check the interceptor bit here or check for the global proxy object? http://codereview.chromium.org/460142/diff/1/3#newcode337 src/ia32/ic-ia32.cc:337: __ shl(edi, kPointerSizeLog2 + 1); You can combine this mov-shl-mov-cmp combination into the operand of a single cmp I think. http://codereview.chromium.org/460142/diff/1/3#newcode344 src/ia32/ic-ia32.cc:344: __ cmp(eax, Operand(edi)); And this cmp could just take the operand instead of using eax as a temp. http://codereview.chromium.org/460142/diff/1/3#newcode347 src/ia32/ic-ia32.cc:347: // Get field offset and check that it is an in-object property. In a later change we could handle out-of-object fast case properties here, right? http://codereview.chromium.org/460142/diff/1/6 File src/x64/ic-x64.cc (right): http://codereview.chromium.org/460142/diff/1/6#newcode53 src/x64/ic-x64.cc:53: Register name, bool check_dictionary) { Use enum. http://codereview.chromium.org/460142 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
