LGTM with comments.
http://codereview.chromium.org/1858002/diff/1/4 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/1858002/diff/1/4#newcode4544 src/x64/codegen-x64.cc:4544: deferred->Branch(less); Should be below, not less, I think, since we do cmpb. http://codereview.chromium.org/1858002/diff/1/4#newcode4556 src/x64/codegen-x64.cc:4556: Condition both_smi = __ CheckBothSmi(index1.reg(), index2.reg()); Shouldn't you check that both are positive smis, and below the size of the array? Or do you know that, because of the caller of this function? If so, it should be commented, I think. I checked and this is not commented in codegen-x64.h. http://codereview.chromium.org/1858002/diff/1/4#newcode4580 src/x64/codegen-x64.cc:4580: // (or them and test against Smi mask.) You could test them separately, and omit one RecordWrite for each, but if both smi or both non-smi is the common case, your idea is better. http://codereview.chromium.org/1858002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
