LGTM.
http://codereview.chromium.org/294022/diff/1002/10 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/294022/diff/1002/10#newcode394 Line 394: __ j(above_equal, &slow); // go to slow if key is negative or ge length. http://codereview.chromium.org/294022/diff/1002/10#newcode441 Line 441: __ j(above_equal, &box_int); I think these four lines are equivalent to __cmp(eax, 0xC0000000) __j(sign, &box_int) If they are copied from somewhere else, that should be changed too. http://codereview.chromium.org/294022/diff/1002/10#newcode688 Line 688: (array_type == kExternalFloatArray) ? taken : not_taken); Hints are disabled by default, and we should probably get rid of them altogether, so you don't need to do them. http://codereview.chromium.org/294022/diff/1002/11 File src/x64/ic-x64.cc (right): http://codereview.chromium.org/294022/diff/1002/11#newcode404 Line 404: __ j(above_equal, &slow); Shouldn't this be a signed comparison? I think we use an unsigned comparison in other keyed ICs because we have earlier checked both the smi tag and the sign bit of the key, leaving only positive smis. Oh, nevermind - it is putting the negative ones up, so they are caught too. Comment this: "Unsigned comparison catches both negative values and values that are too big." http://codereview.chromium.org/294022/diff/1002/11#newcode696 Line 696: __ movq(rdx, Operand(rsp, 2 * kPointerSize)); // 2 ~ return address, key This comment is from a long time ago, and is confusing. Now that we have the ----- S t a t e ----- header, it can be removed. http://codereview.chromium.org/294022/diff/1002/11#newcode782 Line 782: __ fistp_d(Operand(rsp, 0)); Does this do the right thing for NaN and infinities? http://codereview.chromium.org/294022 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
