http://codereview.chromium.org/436001/diff/2001/2023 File src/arm/ic-arm.cc (right):
http://codereview.chromium.org/436001/diff/2001/2023#newcode113 src/arm/ic-arm.cc:113: __ add(t1, t1, Operand(StringDictionary::GetProbeOffset(i))); On 2009/11/24 08:26:37, Erik Corry wrote: > If you add the probe offset shifted left here then you can omit the LSR above > and instead put it in the and instruction below. Done. http://codereview.chromium.org/436001/diff/2001/2022 File src/arm/stub-cache-arm.cc (right): http://codereview.chromium.org/436001/diff/2001/2022#newcode232 src/arm/stub-cache-arm.cc:232: __ ldr(r0, FieldMemOperand(receiver, String::kLengthOffset)); On 2009/11/24 08:26:37, Erik Corry wrote: > isn't this already smi tagged? No. http://codereview.chromium.org/436001/diff/2001/2006 File src/heap.cc (right): http://codereview.chromium.org/436001/diff/2001/2006#newcode1847 src/heap.cc:1847: cons_string->set_hash_field(0); On 2009/11/24 08:26:37, Erik Corry wrote: > we should have a constant that means hash not yet calculated Done added String::kEmptyHashField. http://codereview.chromium.org/436001/diff/2001/2006#newcode2618 src/heap.cc:2618: // Set length and hash field of the allocated string. On 2009/11/24 08:26:37, Erik Corry wrote: > field -> fields Done. http://codereview.chromium.org/436001/diff/2001/2006#newcode2655 src/heap.cc:2655: String::cast(result)->set_hash_field(0); On 2009/11/24 08:26:37, Erik Corry wrote: > And here Done (and in three other places as well). http://codereview.chromium.org/436001/diff/2001/2006#newcode2683 src/heap.cc:2683: String::cast(result)->set_hash_field(0); On 2009/11/24 08:26:37, Erik Corry wrote: > And here Done. http://codereview.chromium.org/436001/diff/2001/2025 File src/heap.h (right): http://codereview.chromium.org/436001/diff/2001/2025#newcode74 src/heap.h:74: V(Map, undetectable_string_map, UndetectableShortStringMap) \ On 2009/11/24 08:26:37, Erik Corry wrote: > why are these still short? No more they are. http://codereview.chromium.org/436001/diff/2001/2012 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/436001/diff/2001/2012#newcode316 src/ia32/ic-ia32.cc:316: // Array index string: If short enough use cache in length/hash field (ebx). On 2009/11/23 13:56:46, Mads Ager wrote: > We need to look for formulations like these and get rid of them. Done, found the same in the x64 version. http://codereview.chromium.org/436001/diff/2001/2012#newcode323 src/ia32/ic-ia32.cc:323: const int kLengthFieldLimit = On 2009/11/23 13:56:46, Mads Ager wrote: > I'm getting confused by the naming here. We are talking about the hash field > here and not the length field, right? I removed the length field limit check which did not make any sense with the new object layout and is not required anyway. Did the same in the x64 version. This part of the keyed load ic is not implemented on ARM. http://codereview.chromium.org/436001/diff/2001/2014 File src/objects.h (right): http://codereview.chromium.org/436001/diff/2001/2014#newcode225 src/objects.h:225: V(SYMBOL_TYPE) \ On 2009/11/23 13:56:46, Mads Ager wrote: > Please align the '\' on the right as before. Done. http://codereview.chromium.org/436001/diff/2001/2014#newcode298 src/objects.h:298: V(SYMBOL_TYPE, \ On 2009/11/23 13:56:46, Mads Ager wrote: > Alignment again. Done. http://codereview.chromium.org/436001/diff/2001/2014#newcode3881 src/objects.h:3881: // For strings which are array indexes the hash value has the On 2009/11/23 13:56:46, Mads Ager wrote: > Complete comment please. I got confused by the ArrayIndexHashLengthShift, but I > see from the implementation why it is there. The comment should explain that. Finished the comment, and changed the line to static const int kArrayIndexHashLengthShift = 24 + kNofLengthBitFields; to show that 24 bits are reserved for the array index value. http://codereview.chromium.org/436001/diff/2001/2016 File src/utils.h (right): http://codereview.chromium.org/436001/diff/2001/2016#newcode588 src/utils.h:588: int TenToThe(int exponent); On 2009/11/24 08:26:37, Erik Corry wrote: > Is this used with a non-constant exponent. For constant exponents the 1e5 > notation is more compact and simpler. It is used with String::kMaxCachedArrayIndexLength (which is currently 7) http://codereview.chromium.org/436001/diff/2001/2019 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/436001/diff/2001/2019#newcode3772 src/x64/codegen-x64.cc:3772: // Check for index out of range. On 2009/11/24 08:26:37, Erik Corry wrote: > Looks like you can omit the movl and just put the field operand in the cmpl Done. http://codereview.chromium.org/436001 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
