Revision: 3449 Author: [email protected] Date: Thu Dec 10 07:10:50 2009 Log: Reapply keyed load cache probing in generated code. I introduced a bug just before committing which broke snapshot builds. The code is nearly identical to the previous submit.
[email protected] Review URL: http://codereview.chromium.org/491004 http://code.google.com/p/v8/source/detail?r=3449 Modified: /branches/bleeding_edge/src/assembler.cc /branches/bleeding_edge/src/assembler.h /branches/bleeding_edge/src/heap.cc /branches/bleeding_edge/src/heap.h /branches/bleeding_edge/src/ia32/ic-ia32.cc /branches/bleeding_edge/src/ic.h /branches/bleeding_edge/src/serialize.cc /branches/bleeding_edge/src/x64/ic-x64.cc ======================================= --- /branches/bleeding_edge/src/assembler.cc Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/assembler.cc Thu Dec 10 07:10:50 2009 @@ -571,6 +571,16 @@ ExternalReference ExternalReference::random_positive_smi_function() { return ExternalReference(Redirect(FUNCTION_ADDR(V8::RandomPositiveSmi))); } + + +ExternalReference ExternalReference::keyed_lookup_cache_keys() { + return ExternalReference(KeyedLookupCache::keys_address()); +} + + +ExternalReference ExternalReference::keyed_lookup_cache_field_offsets() { + return ExternalReference(KeyedLookupCache::field_offsets_address()); +} ExternalReference ExternalReference::the_hole_value_location() { ======================================= --- /branches/bleeding_edge/src/assembler.h Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/assembler.h Thu Dec 10 07:10:50 2009 @@ -401,6 +401,10 @@ static ExternalReference builtin_passed_function(); static ExternalReference random_positive_smi_function(); + // Static data in the keyed lookup cache. + static ExternalReference keyed_lookup_cache_keys(); + static ExternalReference keyed_lookup_cache_field_offsets(); + // Static variable Factory::the_hole_value.location() static ExternalReference the_hole_value_location(); ======================================= --- /branches/bleeding_edge/src/heap.cc Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/heap.cc Thu Dec 10 07:10:50 2009 @@ -3963,8 +3963,8 @@ int KeyedLookupCache::Hash(Map* map, String* name) { // Uses only lower 32 bits if pointers are larger. uintptr_t addr_hash = - static_cast<uint32_t>(reinterpret_cast<uintptr_t>(map)) >> 2; - return (addr_hash ^ name->Hash()) % kLength; + static_cast<uint32_t>(reinterpret_cast<uintptr_t>(map)) >> kMapHashShift; + return (addr_hash ^ name->Hash()) & kCapacityMask; } ======================================= --- /branches/bleeding_edge/src/heap.h Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/heap.h Thu Dec 10 07:10:50 2009 @@ -1300,17 +1300,33 @@ // Clear the cache. static void Clear(); + + static const int kLength = 64; + static const int kCapacityMask = kLength - 1; + static const int kMapHashShift = 2; + private: static inline int Hash(Map* map, String* name); - static const int kLength = 64; + + // Get the address of the keys and field_offsets arrays. Used in + // generated code to perform cache lookups. + static Address keys_address() { + return reinterpret_cast<Address>(&keys_); + } + + static Address field_offsets_address() { + return reinterpret_cast<Address>(&field_offsets_); + } + struct Key { Map* map; String* name; }; static Key keys_[kLength]; static int field_offsets_[kLength]; -}; - + + friend class ExternalReference; +}; // Cache for mapping (array, property name) into descriptor index. ======================================= --- /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec 10 07:10:50 2009 @@ -48,9 +48,13 @@ // must always call a backup property load that is complete. // This function is safe to call if the receiver has fast properties, // or if name is not a symbol, and will jump to the miss_label in that case. -static void GenerateDictionaryLoad(MacroAssembler* masm, Label* miss_label, - Register r0, Register r1, Register r2, - Register name) { +static void GenerateDictionaryLoad(MacroAssembler* masm, + Label* miss_label, + Register r0, + Register r1, + Register r2, + Register name, + DictionaryCheck check_dictionary) { // Register use: // // r0 - used to hold the property dictionary. @@ -86,11 +90,15 @@ __ cmp(r0, JS_BUILTINS_OBJECT_TYPE); __ j(equal, miss_label, not_taken); - // Check that the properties array is a dictionary. + // Load properties array. __ mov(r0, FieldOperand(r1, JSObject::kPropertiesOffset)); - __ cmp(FieldOperand(r0, HeapObject::kMapOffset), - Immediate(Factory::hash_table_map())); - __ j(not_equal, miss_label); + + // Check that the properties array is a dictionary. + if (check_dictionary == CHECK_DICTIONARY) { + __ cmp(FieldOperand(r0, HeapObject::kMapOffset), + Immediate(Factory::hash_table_map())); + __ j(not_equal, miss_label); + } // Compute the capacity mask. const int kCapacityOffset = @@ -223,7 +231,8 @@ // -- esp[4] : name // -- esp[8] : receiver // ----------------------------------- - Label slow, check_string, index_int, index_string, check_pixel_array; + Label slow, check_string, index_int, index_string; + Label check_pixel_array, probe_dictionary; // Load name and receiver. __ mov(eax, Operand(esp, kPointerSize)); @@ -302,17 +311,72 @@ __ test(ebx, Immediate(String::kIsArrayIndexMask)); __ j(not_zero, &index_string, not_taken); - // If the string is a symbol, do a quick inline probe of the receiver's - // dictionary, if it exists. + // Is the string a symbol? __ movzx_b(ebx, FieldOperand(edx, Map::kInstanceTypeOffset)); __ test(ebx, Immediate(kIsSymbolMask)); __ j(zero, &slow, not_taken); - // Probe the dictionary leaving result in ecx. - GenerateDictionaryLoad(masm, &slow, ebx, ecx, edx, eax); + + // If the receiver is a fast-case object, check the keyed lookup + // cache. Otherwise probe the dictionary leaving result in ecx. + __ mov(ebx, FieldOperand(ecx, JSObject::kPropertiesOffset)); + __ cmp(FieldOperand(ebx, HeapObject::kMapOffset), + Immediate(Factory::hash_table_map())); + __ j(equal, &probe_dictionary); + + // Load the map of the receiver, compute the keyed lookup cache hash + // based on 32 bits of the map pointer and the string hash. + __ mov(ebx, FieldOperand(ecx, HeapObject::kMapOffset)); + __ mov(edx, ebx); + __ shr(edx, KeyedLookupCache::kMapHashShift); + __ mov(eax, FieldOperand(eax, String::kHashFieldOffset)); + __ shr(eax, String::kHashShift); + __ xor_(edx, Operand(eax)); + __ and_(edx, KeyedLookupCache::kCapacityMask); + + // Load the key (consisting of map and symbol) from the cache and + // check for match. + ExternalReference cache_keys + = ExternalReference::keyed_lookup_cache_keys(); + __ mov(edi, edx); + __ shl(edi, kPointerSizeLog2 + 1); + __ cmp(ebx, Operand::StaticArray(edi, times_1, cache_keys)); + __ j(not_equal, &slow); + __ add(Operand(edi), Immediate(kPointerSize)); + __ mov(edi, Operand::StaticArray(edi, times_1, cache_keys)); + __ cmp(edi, Operand(esp, kPointerSize)); + __ j(not_equal, &slow); + + // Get field offset and check that it is an in-object property. + ExternalReference cache_field_offsets + = ExternalReference::keyed_lookup_cache_field_offsets(); + __ mov(eax, + Operand::StaticArray(edx, times_pointer_size, cache_field_offsets)); + __ movzx_b(edx, FieldOperand(ebx, Map::kInObjectPropertiesOffset)); + __ cmp(eax, Operand(edx)); + __ j(above_equal, &slow); + + // Load in-object property. + __ sub(eax, Operand(edx)); + __ movzx_b(edx, FieldOperand(ebx, Map::kInstanceSizeOffset)); + __ add(eax, Operand(edx)); + __ mov(eax, FieldOperand(ecx, eax, times_pointer_size, 0)); + __ ret(0); + + // Do a quick inline probe of the receiver's dictionary, if it + // exists. + __ bind(&probe_dictionary); + GenerateDictionaryLoad(masm, + &slow, + ebx, + ecx, + edx, + eax, + DICTIONARY_CHECK_DONE); GenerateCheckNonObjectOrLoaded(masm, &slow, ecx, edx); __ mov(eax, Operand(ecx)); __ IncrementCounter(&Counters::keyed_load_generic_symbol, 1); __ ret(0); + // If the hash field contains an array index pick it out. The assert checks // that the constants for the maximum number of digits for an array index // cached in the hash field and the number of bits reserved for it does not @@ -885,7 +949,7 @@ bool is_global_object, Label* miss) { // Search dictionary - put result in register edx. - GenerateDictionaryLoad(masm, miss, eax, edx, ebx, ecx); + GenerateDictionaryLoad(masm, miss, eax, edx, ebx, ecx, CHECK_DICTIONARY); // Move the result to register edi and check that it isn't a smi. __ mov(edi, Operand(edx)); @@ -1088,7 +1152,7 @@ // Search the dictionary placing the result in eax. __ bind(&probe); - GenerateDictionaryLoad(masm, &miss, edx, eax, ebx, ecx); + GenerateDictionaryLoad(masm, &miss, edx, eax, ebx, ecx, CHECK_DICTIONARY); GenerateCheckNonObjectOrLoaded(masm, &miss, eax, edx); __ ret(0); ======================================= --- /branches/bleeding_edge/src/ic.h Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/ic.h Thu Dec 10 07:10:50 2009 @@ -33,6 +33,11 @@ namespace v8 { namespace internal { +// Flag indicating whether an IC stub needs to check that a backing +// store is in dictionary case. +enum DictionaryCheck { CHECK_DICTIONARY, DICTIONARY_CHECK_DONE }; + + // IC_UTIL_LIST defines all utility functions called from generated // inline caching code. The argument for the macro, ICU, is the function name. #define IC_UTIL_LIST(ICU) \ ======================================= --- /branches/bleeding_edge/src/serialize.cc Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/serialize.cc Thu Dec 10 07:10:50 2009 @@ -484,6 +484,15 @@ 21, "NativeRegExpMacroAssembler::GrowStack()"); #endif + // Keyed lookup cache. + Add(ExternalReference::keyed_lookup_cache_keys().address(), + UNCLASSIFIED, + 22, + "KeyedLookupCache::keys()"); + Add(ExternalReference::keyed_lookup_cache_field_offsets().address(), + UNCLASSIFIED, + 23, + "KeyedLookupCache::field_offsets()"); } ======================================= --- /branches/bleeding_edge/src/x64/ic-x64.cc Thu Dec 10 04:52:28 2009 +++ /branches/bleeding_edge/src/x64/ic-x64.cc Thu Dec 10 07:10:50 2009 @@ -48,9 +48,13 @@ // must always call a backup property load that is complete. // This function is safe to call if the receiver has fast properties, // or if name is not a symbol, and will jump to the miss_label in that case. -static void GenerateDictionaryLoad(MacroAssembler* masm, Label* miss_label, - Register r0, Register r1, Register r2, - Register name) { +static void GenerateDictionaryLoad(MacroAssembler* masm, + Label* miss_label, + Register r0, + Register r1, + Register r2, + Register name, + DictionaryCheck check_dictionary) { // Register use: // // r0 - used to hold the property dictionary. @@ -86,10 +90,14 @@ __ cmpb(r0, Immediate(JS_BUILTINS_OBJECT_TYPE)); __ j(equal, miss_label); - // Check that the properties array is a dictionary. + // Load properties array. __ movq(r0, FieldOperand(r1, JSObject::kPropertiesOffset)); - __ Cmp(FieldOperand(r0, HeapObject::kMapOffset), Factory::hash_table_map()); - __ j(not_equal, miss_label); + + if (check_dictionary == CHECK_DICTIONARY) { + // Check that the properties array is a dictionary. + __ Cmp(FieldOperand(r0, HeapObject::kMapOffset), Factory::hash_table_map()); + __ j(not_equal, miss_label); + } // Compute the capacity mask. const int kCapacityOffset = @@ -246,7 +254,8 @@ // -- rsp[8] : name // -- rsp[16] : receiver // ----------------------------------- - Label slow, check_string, index_int, index_string, check_pixel_array; + Label slow, check_string, index_int, index_string; + Label check_pixel_array, probe_dictionary; // Load name and receiver. __ movq(rax, Operand(rsp, kPointerSize)); @@ -319,14 +328,68 @@ __ movl(rbx, FieldOperand(rax, String::kHashFieldOffset)); __ testl(rbx, Immediate(String::kIsArrayIndexMask)); - // If the string is a symbol, do a quick inline probe of the receiver's - // dictionary, if it exists. + // Is the string a symbol? __ j(not_zero, &index_string); // The value in rbx is used at jump target. __ testb(FieldOperand(rdx, Map::kInstanceTypeOffset), Immediate(kIsSymbolMask)); __ j(zero, &slow); - // Probe the dictionary leaving result in rcx. - GenerateDictionaryLoad(masm, &slow, rbx, rcx, rdx, rax); + + // If the receiver is a fast-case object, check the keyed lookup + // cache. Otherwise probe the dictionary leaving result in rcx. + __ movq(rbx, FieldOperand(rcx, JSObject::kPropertiesOffset)); + __ Cmp(FieldOperand(rbx, HeapObject::kMapOffset), Factory::hash_table_map()); + __ j(equal, &probe_dictionary); + + // Load the map of the receiver, compute the keyed lookup cache hash + // based on 32 bits of the map pointer and the string hash. + __ movq(rbx, FieldOperand(rcx, HeapObject::kMapOffset)); + __ movl(rdx, rbx); + __ shr(rdx, Immediate(KeyedLookupCache::kMapHashShift)); + __ movl(rax, FieldOperand(rax, String::kHashFieldOffset)); + __ shr(rax, Immediate(String::kHashShift)); + __ xor_(rdx, rax); + __ and_(rdx, Immediate(KeyedLookupCache::kCapacityMask)); + + // Load the key (consisting of map and symbol) from the cache and + // check for match. + ExternalReference cache_keys + = ExternalReference::keyed_lookup_cache_keys(); + __ movq(rdi, rdx); + __ shl(rdi, Immediate(kPointerSizeLog2 + 1)); + __ movq(kScratchRegister, cache_keys); + __ cmpq(rbx, Operand(kScratchRegister, rdi, times_1, 0)); + __ j(not_equal, &slow); + __ movq(rdi, Operand(kScratchRegister, rdi, times_1, kPointerSize)); + __ cmpq(Operand(rsp, kPointerSize), rdi); + __ j(not_equal, &slow); + + // Get field offset which is a 32-bit integer and check that it is + // an in-object property. + ExternalReference cache_field_offsets + = ExternalReference::keyed_lookup_cache_field_offsets(); + __ movq(kScratchRegister, cache_field_offsets); + __ movl(rax, Operand(kScratchRegister, rdx, times_4, 0)); + __ movzxbq(rdx, FieldOperand(rbx, Map::kInObjectPropertiesOffset)); + __ cmpq(rax, rdx); + __ j(above_equal, &slow); + + // Load in-object property. + __ subq(rax, rdx); + __ movzxbq(rdx, FieldOperand(rbx, Map::kInstanceSizeOffset)); + __ addq(rax, rdx); + __ movq(rax, FieldOperand(rcx, rax, times_pointer_size, 0)); + __ ret(0); + + // Do a quick inline probe of the receiver's dictionary, if it + // exists. + __ bind(&probe_dictionary); + GenerateDictionaryLoad(masm, + &slow, + rbx, + rcx, + rdx, + rax, + DICTIONARY_CHECK_DONE); GenerateCheckNonObjectOrLoaded(masm, &slow, rcx); __ movq(rax, rcx); __ IncrementCounter(&Counters::keyed_load_generic_symbol, 1); @@ -971,8 +1034,8 @@ int argc, bool is_global_object, Label* miss) { - // Search dictionary - put result in register edx. - GenerateDictionaryLoad(masm, miss, rax, rdx, rbx, rcx); + // Search dictionary - put result in register rdx. + GenerateDictionaryLoad(masm, miss, rax, rdx, rbx, rcx, CHECK_DICTIONARY); // Move the result to register rdi and check that it isn't a smi. __ movq(rdi, rdx); @@ -1196,9 +1259,9 @@ Immediate(1 << Map::kIsAccessCheckNeeded)); __ j(not_zero, &miss); - // Search the dictionary placing the result in eax. + // Search the dictionary placing the result in rax. __ bind(&probe); - GenerateDictionaryLoad(masm, &miss, rdx, rax, rbx, rcx); + GenerateDictionaryLoad(masm, &miss, rdx, rax, rbx, rcx, CHECK_DICTIONARY); GenerateCheckNonObjectOrLoaded(masm, &miss, rax); __ ret(0); -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
