LGTM w/ comments.
http://codereview.chromium.org/209048/diff/1/2 File src/x64/builtins-x64.cc (right): http://codereview.chromium.org/209048/diff/1/2#newcode726 Line 726: __ cmpq(rdx, Immediate(JSObject::kInitialMaxFastElementArray << kSmiTagSize)); Good. In the long run there will be a SmiCompare that allows equal/less_than etc. to be used, but for now this should suffice. http://codereview.chromium.org/209048/diff/4002/4005 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/209048/diff/4002/4005#newcode525 Line 525: if (Smi::IsValid(constant)) { Drop the IsValid test, or change it to an ASSERT (you can be greater than a non-valid smi, so unlike SmiEqualsConstant, it doesn't rule out that the test should succeede). http://codereview.chromium.org/209048/diff/4002/4005#newcode526 Line 526: Condition are_greater_equal = CheckSmiEqualsConstant(src, constant); This doesn't work. SmiEqualsConstant only checks for equality (as far as you can assume). Do the test inline (there is currently no smi compare, but there will be eventually), so: cmpl(src, constant << kSmiTagSize); j(greater_equal, on_greater_equals); http://codereview.chromium.org/209048 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
