LGTM with a couple of nits.
http://codereview.chromium.org/2853003/diff/1/13 File src/arm/ic-arm.cc (right): http://codereview.chromium.org/2853003/diff/1/13#newcode178 src/arm/ic-arm.cc:178: // key - holds the smi key on entry and is unchanged. The comments here are a bit confusing since |key| is not unchanged in one of the usages of this because |key| and |result| are the same register at that use site. http://codereview.chromium.org/2853003/diff/1/13#newcode335 src/arm/ic-arm.cc:335: Register result, The comment says that receiver and key are both unchanged. However, it seems that this is sometimes used where receiver or key is the same register as result. In that case, receiver or key is changed. Can we remove the result register argument and make it clear in the comment on register usage that the receiver is overwritten with the result if the load succeeds and is unchanged otherwise? Probably similar for the other platforms. http://codereview.chromium.org/2853003/diff/1/13#newcode355 src/arm/ic-arm.cc:355: // t2 - used for the index into the dictionary. This line doesn't seem to belong here? :) http://codereview.chromium.org/2853003/diff/1/13#newcode371 src/arm/ic-arm.cc:371: __ ldr(scratch2, Why do you need |scratch2| here? Based on the comment in this function it would make sense to load directly to |result| here. Remove the |result| argument to make the overwriting clear? If that is not possible, at least update the comments to state that result is unchanged unless the load succeeds. http://codereview.chromium.org/2853003/diff/1/13#newcode392 src/arm/ic-arm.cc:392: // r0: key Remove this line. |key| is passed as an argument. http://codereview.chromium.org/2853003/diff/1/13#newcode704 src/arm/ic-arm.cc:704: // Check that the value n r1 is a JSFunction. n -> in http://codereview.chromium.org/2853003/diff/1/2 File test/mjsunit/keyed-call-generic.js (right): http://codereview.chromium.org/2853003/diff/1/2#newcode31 test/mjsunit/keyed-call-generic.js:31: // for(var i = 0; i != 10; i++ ) { Code in comment. http://codereview.chromium.org/2853003/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
