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); On 2010/05/03 14:59:52, William Hesse wrote:
Should be below, not less, I think, since we do cmpb.
Yes, good catch, thanks. http://codereview.chromium.org/1858002/diff/1/4#newcode4556 src/x64/codegen-x64.cc:4556: Condition both_smi = __ CheckBothSmi(index1.reg(), index2.reg()); On 2010/05/03 14:59:52, William Hesse wrote:
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.
I know because of the caller. This inlined code is only generated for the sort routine in array.js. I'll add a comment to both the ia32 and x64 versions. The indices are expected to be positive and within bounds - there are no bounds checks in this code either. http://codereview.chromium.org/1858002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
