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); On 2009/10/21 08:55:20, William Hesse wrote: > // go to slow if key is negative or ge length. Done. http://codereview.chromium.org/294022/diff/1002/10#newcode394 Line 394: __ j(above_equal, &slow); On 2009/10/21 09:18:40, Christian Plesner Hansen wrote: > What about negative indices? They are handled via the unsigned (above_equal) comparison -- we go to the slow case. http://codereview.chromium.org/294022/diff/1002/10#newcode421 Line 421: ASSERT(false); On 2009/10/21 09:18:40, Christian Plesner Hansen wrote: > UNREACHABLE() Done. http://codereview.chromium.org/294022/diff/1002/10#newcode441 Line 441: __ j(above_equal, &box_int); On 2009/10/21 08:55:20, William Hesse wrote: > 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. Per the comment, this is the assembly translation of Smi::IsValid. Since I know this instruction sequence works I'm going to leave it as is. http://codereview.chromium.org/294022/diff/1002/10#newcode443 Line 443: ASSERT(array_type == kExternalUnsignedIntArray); On 2009/10/21 09:18:40, Christian Plesner Hansen wrote: > ASSERT_EQ Done. http://codereview.chromium.org/294022/diff/1002/10#newcode679 Line 679: __ j(above_equal, &slow); On 2009/10/21 09:18:40, Christian Plesner Hansen wrote: > Negative? Handled by this comparison. See above. http://codereview.chromium.org/294022/diff/1002/10#newcode688 Line 688: (array_type == kExternalFloatArray) ? taken : not_taken); On 2009/10/21 08:55:20, William Hesse wrote: > Hints are disabled by default, and we should probably get rid of them > altogether, so you don't need to do them. OK. They aren't present in the 64-bit port anyway. http://codereview.chromium.org/294022/diff/1002/10#newcode726 Line 726: __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); On 2009/10/21 09:18:40, Christian Plesner Hansen wrote: > Please add a comment about how this handles NaN, Infinity and other odd values. Done. Their cast value is not specified. 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); On 2009/10/21 08:55:20, William Hesse wrote: > 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." Done. http://codereview.chromium.org/294022/diff/1002/11#newcode696 Line 696: __ movq(rdx, Operand(rsp, 2 * kPointerSize)); // 2 ~ return address, key On 2009/10/21 08:55:20, William Hesse wrote: > 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. Done. http://codereview.chromium.org/294022/diff/1002/11#newcode782 Line 782: __ fistp_d(Operand(rsp, 0)); On 2009/10/21 08:55:20, William Hesse wrote: > Does this do the right thing for NaN and infinities? These arrays don't specify what happens for those values, so the error behavior should be acceptable. http://codereview.chromium.org/294022 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
