Revision: 3446 Author: [email protected] Date: Thu Dec 10 04:52:28 2009 Log: Revert keyed load cache probing in generated code.
Crashes on Windows. [email protected] Review URL: http://codereview.chromium.org/488006 http://code.google.com/p/v8/source/detail?r=3446 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/assembler-ia32.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 01:21:23 2009 +++ /branches/bleeding_edge/src/assembler.cc Thu Dec 10 04:52:28 2009 @@ -571,16 +571,6 @@ 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 01:21:23 2009 +++ /branches/bleeding_edge/src/assembler.h Thu Dec 10 04:52:28 2009 @@ -401,10 +401,6 @@ 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 01:21:23 2009 +++ /branches/bleeding_edge/src/heap.cc Thu Dec 10 04:52:28 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)) >> kMapHashShift; - return (addr_hash ^ name->Hash()) & kCapacityMask; + static_cast<uint32_t>(reinterpret_cast<uintptr_t>(map)) >> 2; + return (addr_hash ^ name->Hash()) % kLength; } ======================================= --- /branches/bleeding_edge/src/heap.h Thu Dec 10 01:21:23 2009 +++ /branches/bleeding_edge/src/heap.h Thu Dec 10 04:52:28 2009 @@ -1300,35 +1300,19 @@ // 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); - - // 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_); - } - + static const int kLength = 64; 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. // The cache contains both positive and negative results. // Descriptor index equals kNotFound means the property is absent. ======================================= --- /branches/bleeding_edge/src/ia32/assembler-ia32.h Thu Dec 10 01:21:23 2009 +++ /branches/bleeding_edge/src/ia32/assembler-ia32.h Thu Dec 10 04:52:28 2009 @@ -230,7 +230,6 @@ times_4 = 2, times_8 = 3, times_pointer_size = times_4, - two_times_pointer_size = times_8, times_half_pointer_size = times_2 }; @@ -272,17 +271,6 @@ return Operand(index, scale, reinterpret_cast<int32_t>(arr.address()), RelocInfo::EXTERNAL_REFERENCE); } - - static Operand StaticArray(Register index, - ScaleFactor scale, - const ExternalReference& arr, - int32_t disp) { - return Operand(index, - scale, - reinterpret_cast<int32_t>(arr.address() + disp), - RelocInfo::EXTERNAL_REFERENCE); - } - // Returns true if this Operand is a wrapper for the specified register. bool is_reg(Register reg) const; ======================================= --- /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec 10 01:21:23 2009 +++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec 10 04:52:28 2009 @@ -48,13 +48,9 @@ // 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, - DictionaryCheck check_dictionary) { +static void GenerateDictionaryLoad(MacroAssembler* masm, Label* miss_label, + Register r0, Register r1, Register r2, + Register name) { // Register use: // // r0 - used to hold the property dictionary. @@ -90,15 +86,11 @@ __ cmp(r0, JS_BUILTINS_OBJECT_TYPE); __ j(equal, miss_label, not_taken); - // Load properties array. + // Check that the properties array is a dictionary. __ mov(r0, FieldOperand(r1, JSObject::kPropertiesOffset)); - - // 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); - } + __ cmp(FieldOperand(r0, HeapObject::kMapOffset), + Immediate(Factory::hash_table_map())); + __ j(not_equal, miss_label); // Compute the capacity mask. const int kCapacityOffset = @@ -231,8 +223,7 @@ // -- esp[4] : name // -- esp[8] : receiver // ----------------------------------- - Label slow, check_string, index_int, index_string; - Label check_pixel_array, probe_dictionary; + Label slow, check_string, index_int, index_string, check_pixel_array; // Load name and receiver. __ mov(eax, Operand(esp, kPointerSize)); @@ -311,72 +302,17 @@ __ test(ebx, Immediate(String::kIsArrayIndexMask)); __ j(not_zero, &index_string, not_taken); - // Is the string a symbol? + // If the string is a symbol, do a quick inline probe of the receiver's + // dictionary, if it exists. __ movzx_b(ebx, FieldOperand(edx, Map::kInstanceTypeOffset)); __ test(ebx, Immediate(kIsSymbolMask)); __ j(zero, &slow, not_taken); - - // 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(); - __ cmp(ebx, Operand::StaticArray(edx, two_times_pointer_size, cache_keys)); - __ j(not_equal, &slow); - __ mov(edi, Operand::StaticArray(edx, - two_times_pointer_size, - cache_keys, - kPointerSize)); - __ 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); + // Probe the dictionary leaving result in ecx. + GenerateDictionaryLoad(masm, &slow, ebx, ecx, edx, eax); 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 @@ -949,7 +885,7 @@ bool is_global_object, Label* miss) { // Search dictionary - put result in register edx. - GenerateDictionaryLoad(masm, miss, eax, edx, ebx, ecx, CHECK_DICTIONARY); + GenerateDictionaryLoad(masm, miss, eax, edx, ebx, ecx); // Move the result to register edi and check that it isn't a smi. __ mov(edi, Operand(edx)); @@ -1152,7 +1088,7 @@ // Search the dictionary placing the result in eax. __ bind(&probe); - GenerateDictionaryLoad(masm, &miss, edx, eax, ebx, ecx, CHECK_DICTIONARY); + GenerateDictionaryLoad(masm, &miss, edx, eax, ebx, ecx); GenerateCheckNonObjectOrLoaded(masm, &miss, eax, edx); __ ret(0); ======================================= --- /branches/bleeding_edge/src/ic.h Thu Dec 10 01:21:23 2009 +++ /branches/bleeding_edge/src/ic.h Thu Dec 10 04:52:28 2009 @@ -33,11 +33,6 @@ 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 01:21:23 2009 +++ /branches/bleeding_edge/src/serialize.cc Thu Dec 10 04:52:28 2009 @@ -484,15 +484,6 @@ 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 01:21:23 2009 +++ /branches/bleeding_edge/src/x64/ic-x64.cc Thu Dec 10 04:52:28 2009 @@ -48,13 +48,9 @@ // 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, - DictionaryCheck check_dictionary) { +static void GenerateDictionaryLoad(MacroAssembler* masm, Label* miss_label, + Register r0, Register r1, Register r2, + Register name) { // Register use: // // r0 - used to hold the property dictionary. @@ -90,14 +86,10 @@ __ cmpb(r0, Immediate(JS_BUILTINS_OBJECT_TYPE)); __ j(equal, miss_label); - // Load properties array. + // Check that the properties array is a dictionary. __ movq(r0, FieldOperand(r1, JSObject::kPropertiesOffset)); - - 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); - } + __ Cmp(FieldOperand(r0, HeapObject::kMapOffset), Factory::hash_table_map()); + __ j(not_equal, miss_label); // Compute the capacity mask. const int kCapacityOffset = @@ -254,8 +246,7 @@ // -- rsp[8] : name // -- rsp[16] : receiver // ----------------------------------- - Label slow, check_string, index_int, index_string; - Label check_pixel_array, probe_dictionary; + Label slow, check_string, index_int, index_string, check_pixel_array; // Load name and receiver. __ movq(rax, Operand(rsp, kPointerSize)); @@ -328,68 +319,14 @@ __ movl(rbx, FieldOperand(rax, String::kHashFieldOffset)); __ testl(rbx, Immediate(String::kIsArrayIndexMask)); - // Is the string a symbol? + // If the string is a symbol, do a quick inline probe of the receiver's + // dictionary, if it exists. __ j(not_zero, &index_string); // The value in rbx is used at jump target. __ testb(FieldOperand(rdx, Map::kInstanceTypeOffset), Immediate(kIsSymbolMask)); __ j(zero, &slow); - - // 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); + // Probe the dictionary leaving result in rcx. + GenerateDictionaryLoad(masm, &slow, rbx, rcx, rdx, rax); GenerateCheckNonObjectOrLoaded(masm, &slow, rcx); __ movq(rax, rcx); __ IncrementCounter(&Counters::keyed_load_generic_symbol, 1); @@ -1034,8 +971,8 @@ int argc, bool is_global_object, Label* miss) { - // Search dictionary - put result in register rdx. - GenerateDictionaryLoad(masm, miss, rax, rdx, rbx, rcx, CHECK_DICTIONARY); + // Search dictionary - put result in register edx. + GenerateDictionaryLoad(masm, miss, rax, rdx, rbx, rcx); // Move the result to register rdi and check that it isn't a smi. __ movq(rdi, rdx); @@ -1259,9 +1196,9 @@ Immediate(1 << Map::kIsAccessCheckNeeded)); __ j(not_zero, &miss); - // Search the dictionary placing the result in rax. + // Search the dictionary placing the result in eax. __ bind(&probe); - GenerateDictionaryLoad(masm, &miss, rdx, rax, rbx, rcx, CHECK_DICTIONARY); + GenerateDictionaryLoad(masm, &miss, rdx, rax, rbx, rcx); GenerateCheckNonObjectOrLoaded(masm, &miss, rax); __ ret(0); -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
