From 01206ad5bd3939227c10e1b40746c1877591a2a0 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Thu, 9 Oct 2014 14:30:44 +0000 Subject: [PATCH] Add MEGAMORPHIC state support for KeyedStoreIC R=mvstanton@chromium.org Review URL: https://codereview.chromium.org/584043002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24500 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/builtins.cc | 14 ++++++++++++-- src/builtins.h | 3 +++ src/ic/arm/ic-arm.cc | 29 +++++++++++++++++++++++++---- src/ic/arm64/ic-arm64.cc | 30 +++++++++++++++++++++++++++--- src/ic/ia32/ic-ia32.cc | 31 +++++++++++++++++++++++++++---- src/ic/ic.cc | 33 +++++++++++++++++++++------------ src/ic/ic.h | 11 +++++++++-- src/ic/x64/ic-x64.cc | 30 ++++++++++++++++++++++++++---- src/ic/x64/stub-cache-x64.cc | 8 ++++---- 9 files changed, 154 insertions(+), 35 deletions(-) diff --git a/src/builtins.cc b/src/builtins.cc index c52d228..4d2fa0a 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -1314,13 +1314,23 @@ static void Generate_StoreIC_Setter_ForDeopt(MacroAssembler* masm) { } +static void Generate_KeyedStoreIC_Megamorphic(MacroAssembler* masm) { + KeyedStoreIC::GenerateGeneric(masm, SLOPPY, kMissOnMissingHandler); +} + + +static void Generate_KeyedStoreIC_Megamorphic_Strict(MacroAssembler* masm) { + KeyedStoreIC::GenerateGeneric(masm, STRICT, kMissOnMissingHandler); +} + + static void Generate_KeyedStoreIC_Generic(MacroAssembler* masm) { - KeyedStoreIC::GenerateGeneric(masm, SLOPPY); + KeyedStoreIC::GenerateGeneric(masm, SLOPPY, kCallRuntimeOnMissingHandler); } static void Generate_KeyedStoreIC_Generic_Strict(MacroAssembler* masm) { - KeyedStoreIC::GenerateGeneric(masm, STRICT); + KeyedStoreIC::GenerateGeneric(masm, STRICT, kCallRuntimeOnMissingHandler); } diff --git a/src/builtins.h b/src/builtins.h index c1ed91d..da1ae10 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -95,12 +95,15 @@ enum BuiltinExtraArguments { V(KeyedStoreIC_Initialize, KEYED_STORE_IC, UNINITIALIZED, kNoExtraICState) \ V(KeyedStoreIC_PreMonomorphic, KEYED_STORE_IC, PREMONOMORPHIC, \ kNoExtraICState) \ + V(KeyedStoreIC_Megamorphic, KEYED_STORE_IC, MEGAMORPHIC, kNoExtraICState) \ V(KeyedStoreIC_Generic, KEYED_STORE_IC, GENERIC, kNoExtraICState) \ \ V(KeyedStoreIC_Initialize_Strict, KEYED_STORE_IC, UNINITIALIZED, \ StoreIC::kStrictModeState) \ V(KeyedStoreIC_PreMonomorphic_Strict, KEYED_STORE_IC, PREMONOMORPHIC, \ StoreIC::kStrictModeState) \ + V(KeyedStoreIC_Megamorphic_Strict, KEYED_STORE_IC, MEGAMORPHIC, \ + StoreIC::kStrictModeState) \ V(KeyedStoreIC_Generic_Strict, KEYED_STORE_IC, GENERIC, \ StoreIC::kStrictModeState) \ V(KeyedStoreIC_SloppyArguments, KEYED_STORE_IC, MONOMORPHIC, \ diff --git a/src/ic/arm/ic-arm.cc b/src/ic/arm/ic-arm.cc index ae13161..d26dc49 100644 --- a/src/ic/arm/ic-arm.cc +++ b/src/ic/arm/ic-arm.cc @@ -765,8 +765,9 @@ static void KeyedStoreGenerateGenericHelper( } -void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, - StrictMode strict_mode) { +void KeyedStoreIC::GenerateGeneric( + MacroAssembler* masm, StrictMode strict_mode, + KeyedStoreStubCacheRequirement handler_requirement) { // ---------- S t a t e -------------- // -- r0 : value // -- r1 : key @@ -775,7 +776,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // ----------------------------------- Label slow, fast_object, fast_object_grow; Label fast_double, fast_double_grow; - Label array, extra, check_if_double_array; + Label array, extra, check_if_double_array, maybe_name_key, miss; // Register usage. Register value = StoreDescriptor::ValueRegister(); @@ -790,7 +791,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // r4 and r5 are used as general scratch registers. // Check that the key is a smi. - __ JumpIfNotSmi(key, &slow); + __ JumpIfNotSmi(key, &maybe_name_key); // Check that the object isn't a smi. __ JumpIfSmi(receiver, &slow); // Get the map of the object. @@ -822,6 +823,23 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // r1: key. // r2: receiver. PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode); + // Never returns to here. + + __ bind(&maybe_name_key); + __ ldr(r4, FieldMemOperand(key, HeapObject::kMapOffset)); + __ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset)); + __ JumpIfNotUniqueNameInstanceType(r4, &slow); + Code::Flags flags = Code::RemoveTypeAndHolderFromFlags( + Code::ComputeHandlerFlags(Code::STORE_IC)); + masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver, + key, r3, r4, r5, r6); + // Cache miss. + if (handler_requirement == kCallRuntimeOnMissingHandler) { + __ b(&slow); + } else { + DCHECK(handler_requirement == kMissOnMissingHandler); + __ b(&miss); + } // Extra capacity case: Check if there is extra capacity to // perform the store and update the length. Used for adding one @@ -863,6 +881,9 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, &slow, kDontCheckMap, kIncrementLength, value, key, receiver, receiver_map, elements_map, elements); + + __ bind(&miss); + GenerateMiss(masm); } diff --git a/src/ic/arm64/ic-arm64.cc b/src/ic/arm64/ic-arm64.cc index 76f9c24..4ec76b66 100644 --- a/src/ic/arm64/ic-arm64.cc +++ b/src/ic/arm64/ic-arm64.cc @@ -798,8 +798,9 @@ static void KeyedStoreGenerateGenericHelper( } -void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, - StrictMode strict_mode) { +void KeyedStoreIC::GenerateGeneric( + MacroAssembler* masm, StrictMode strict_mode, + KeyedStoreStubCacheRequirement handler_requirement) { ASM_LOCATION("KeyedStoreIC::GenerateGeneric"); Label slow; Label array; @@ -808,6 +809,8 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, Label fast_object_grow; Label fast_double_grow; Label fast_double; + Label maybe_name_key; + Label miss; Register value = StoreDescriptor::ValueRegister(); Register key = StoreDescriptor::NameRegister(); @@ -820,7 +823,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, Register elements = x4; Register elements_map = x5; - __ JumpIfNotSmi(key, &slow); + __ JumpIfNotSmi(key, &maybe_name_key); __ JumpIfSmi(receiver, &slow); __ Ldr(receiver_map, FieldMemOperand(receiver, HeapObject::kMapOffset)); @@ -853,7 +856,23 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // x1: key // x2: receiver PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode); + // Never returns to here. + __ bind(&maybe_name_key); + __ Ldr(x10, FieldMemOperand(key, HeapObject::kMapOffset)); + __ Ldrb(x10, FieldMemOperand(x10, Map::kInstanceTypeOffset)); + __ JumpIfNotUniqueNameInstanceType(x10, &slow); + Code::Flags flags = Code::RemoveTypeAndHolderFromFlags( + Code::ComputeHandlerFlags(Code::STORE_IC)); + masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver, + key, x3, x4, x5, x6); + // Cache miss. + if (handler_requirement == kCallRuntimeOnMissingHandler) { + __ B(&slow); + } else { + DCHECK(handler_requirement == kMissOnMissingHandler); + __ B(&miss); + } __ Bind(&extra); // Extra capacity case: Check if there is extra capacity to @@ -895,6 +914,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, &slow, kDontCheckMap, kIncrementLength, value, key, receiver, receiver_map, elements_map, elements); + + if (handler_requirement == kMissOnMissingHandler) { + __ bind(&miss); + GenerateMiss(masm); + } } diff --git a/src/ic/ia32/ic-ia32.cc b/src/ic/ia32/ic-ia32.cc index 67247d2..13f17eb 100644 --- a/src/ic/ia32/ic-ia32.cc +++ b/src/ic/ia32/ic-ia32.cc @@ -672,12 +672,13 @@ static void KeyedStoreGenerateGenericHelper( } -void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, - StrictMode strict_mode) { +void KeyedStoreIC::GenerateGeneric( + MacroAssembler* masm, StrictMode strict_mode, + KeyedStoreStubCacheRequirement handler_requirement) { // Return address is on the stack. Label slow, fast_object, fast_object_grow; Label fast_double, fast_double_grow; - Label array, extra, check_if_double_array; + Label array, extra, check_if_double_array, maybe_name_key, miss; Register receiver = StoreDescriptor::ReceiverRegister(); Register key = StoreDescriptor::NameRegister(); DCHECK(receiver.is(edx)); @@ -693,7 +694,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, 1 << Map::kIsAccessCheckNeeded | 1 << Map::kIsObserved); __ j(not_zero, &slow); // Check that the key is a smi. - __ JumpIfNotSmi(key, &slow); + __ JumpIfNotSmi(key, &maybe_name_key); __ CmpInstanceType(edi, JS_ARRAY_TYPE); __ j(equal, &array); // Check that the object is some kind of JSObject. @@ -711,6 +712,23 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // Slow case: call runtime. __ bind(&slow); PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode); + // Never returns to here. + + __ bind(&maybe_name_key); + __ mov(ebx, FieldOperand(key, HeapObject::kMapOffset)); + __ movzx_b(ebx, FieldOperand(ebx, Map::kInstanceTypeOffset)); + __ JumpIfNotUniqueNameInstanceType(ebx, &slow); + Code::Flags flags = Code::RemoveTypeAndHolderFromFlags( + Code::ComputeHandlerFlags(Code::STORE_IC)); + masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver, + key, ebx, no_reg); + // Cache miss. + if (handler_requirement == kCallRuntimeOnMissingHandler) { + __ jmp(&slow); + } else { + DCHECK(handler_requirement == kMissOnMissingHandler); + __ jmp(&miss); + } // Extra capacity case: Check if there is extra capacity to // perform the store and update the length. Used for adding one @@ -753,6 +771,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, kCheckMap, kDontIncrementLength); KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow, &slow, kDontCheckMap, kIncrementLength); + + if (handler_requirement == kMissOnMissingHandler) { + __ bind(&miss); + GenerateMiss(masm); + } } diff --git a/src/ic/ic.cc b/src/ic/ic.cc index 63f2ffa..6b47f03 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -801,19 +801,27 @@ void IC::PatchCache(Handle name, Handle code) { case POLYMORPHIC: if (!target()->is_keyed_stub() || state() == PROTOTYPE_FAILURE) { if (UpdatePolymorphicIC(name, code)) break; + // For keyed stubs, we can't know whether old handlers were for the + // same key. CopyICToMegamorphicCache(name); } set_target(*megamorphic_stub()); // Fall through. case MEGAMORPHIC: UpdateMegamorphicCache(*receiver_type(), *name, *code); + // Indicate that we've handled this case. + target_set_ = true; break; case DEBUG_STUB: break; case DEFAULT: - case GENERIC: UNREACHABLE(); break; + case GENERIC: + // The generic keyed store stub re-uses store handlers, which can miss. + // That's ok, no reason to do anything. + DCHECK(target()->kind() == Code::KEYED_STORE_IC); + break; } } @@ -894,7 +902,8 @@ void LoadIC::UpdateCaches(LookupIterator* lookup) { void IC::UpdateMegamorphicCache(HeapType* type, Name* name, Code* code) { - if (kind() == Code::KEYED_LOAD_IC || kind() == Code::KEYED_STORE_IC) return; + // Megamorphic state isn't implemented for keyed loads currently. + if (kind() == Code::KEYED_LOAD_IC) return; Map* map = *TypeToMap(type, isolate()); isolate()->stub_cache()->Set(name, map, code); } @@ -1365,9 +1374,9 @@ Handle StoreIC::megamorphic_stub() { } else { DCHECK(kind() == Code::KEYED_STORE_IC); if (strict_mode() == STRICT) { - return isolate()->builtins()->KeyedStoreIC_Generic_Strict(); + return isolate()->builtins()->KeyedStoreIC_Megamorphic_Strict(); } else { - return isolate()->builtins()->KeyedStoreIC_Generic(); + return isolate()->builtins()->KeyedStoreIC_Megamorphic(); } } } @@ -1813,12 +1822,12 @@ MaybeHandle KeyedStoreIC::Store(Handle object, StoreIC::Store(object, Handle::cast(key), value, JSReceiver::MAY_BE_STORE_FROM_KEYED), Object); - // TODO(jkummerow): Ideally we'd wrap this in "if (!is_target_set())", - // but doing so causes Hydrogen crashes. Needs investigation. - TRACE_GENERIC_IC(isolate(), "KeyedStoreIC", - "unhandled internalized string key"); - TRACE_IC("StoreIC", key); - set_target(*stub); + if (!is_target_set()) { + TRACE_GENERIC_IC(isolate(), "KeyedStoreIC", + "unhandled internalized string key"); + TRACE_IC("StoreIC", key); + set_target(*stub); + } return store_handle; } @@ -2088,7 +2097,7 @@ RUNTIME_FUNCTION(StoreIC_Miss) { DCHECK(args.length() == 3); StoreIC ic(IC::NO_EXTRA_FRAME, isolate); Handle receiver = args.at(0); - Handle key = args.at(1); + Handle key = args.at(1); ic.UpdateState(receiver, key); Handle result; ASSIGN_RETURN_FAILURE_ON_EXCEPTION( @@ -2103,7 +2112,7 @@ RUNTIME_FUNCTION(StoreIC_MissFromStubFailure) { DCHECK(args.length() == 3 || args.length() == 4); StoreIC ic(IC::EXTRA_CALL_FRAME, isolate); Handle receiver = args.at(0); - Handle key = args.at(1); + Handle key = args.at(1); ic.UpdateState(receiver, key); Handle result; ASSIGN_RETURN_FAILURE_ON_EXCEPTION( diff --git a/src/ic/ic.h b/src/ic/ic.h index 295d9b8..f96c6f0 100644 --- a/src/ic/ic.h +++ b/src/ic/ic.h @@ -223,7 +223,6 @@ class IC { return target_maps_.length() > 0 ? *target_maps_.at(0) : NULL; } - protected: inline void UpdateTarget(); private: @@ -526,6 +525,12 @@ enum KeyedStoreCheckMap { kDontCheckMap, kCheckMap }; enum KeyedStoreIncrementLength { kDontIncrementLength, kIncrementLength }; +enum KeyedStoreStubCacheRequirement { + kCallRuntimeOnMissingHandler, + kMissOnMissingHandler +}; + + class KeyedStoreIC : public StoreIC { public: // ExtraICState bits (building on IC) @@ -559,7 +564,9 @@ class KeyedStoreIC : public StoreIC { } static void GenerateMiss(MacroAssembler* masm); static void GenerateSlow(MacroAssembler* masm); - static void GenerateGeneric(MacroAssembler* masm, StrictMode strict_mode); + static void GenerateGeneric( + MacroAssembler* masm, StrictMode strict_mode, + KeyedStoreStubCacheRequirement handler_requirement); static void GenerateSloppyArguments(MacroAssembler* masm); protected: diff --git a/src/ic/x64/ic-x64.cc b/src/ic/x64/ic-x64.cc index ad79f30..db540b8 100644 --- a/src/ic/x64/ic-x64.cc +++ b/src/ic/x64/ic-x64.cc @@ -566,12 +566,13 @@ static void KeyedStoreGenerateGenericHelper( } -void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, - StrictMode strict_mode) { +void KeyedStoreIC::GenerateGeneric( + MacroAssembler* masm, StrictMode strict_mode, + KeyedStoreStubCacheRequirement handler_requirement) { // Return address is on the stack. Label slow, slow_with_tagged_index, fast_object, fast_object_grow; Label fast_double, fast_double_grow; - Label array, extra, check_if_double_array; + Label array, extra, check_if_double_array, maybe_name_key, miss; Register receiver = StoreDescriptor::ReceiverRegister(); Register key = StoreDescriptor::NameRegister(); DCHECK(receiver.is(rdx)); @@ -587,7 +588,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, Immediate(1 << Map::kIsAccessCheckNeeded | 1 << Map::kIsObserved)); __ j(not_zero, &slow_with_tagged_index); // Check that the key is a smi. - __ JumpIfNotSmi(key, &slow_with_tagged_index); + __ JumpIfNotSmi(key, &maybe_name_key); __ SmiToInteger32(key, key); __ CmpInstanceType(r9, JS_ARRAY_TYPE); @@ -610,6 +611,22 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode); // Never returns to here. + __ bind(&maybe_name_key); + __ movp(r9, FieldOperand(key, HeapObject::kMapOffset)); + __ movzxbp(r9, FieldOperand(r9, Map::kInstanceTypeOffset)); + __ JumpIfNotUniqueNameInstanceType(r9, &slow_with_tagged_index); + Code::Flags flags = Code::RemoveTypeAndHolderFromFlags( + Code::ComputeHandlerFlags(Code::STORE_IC)); + masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver, + key, rbx, no_reg); + // Cache miss. + if (handler_requirement == kCallRuntimeOnMissingHandler) { + __ jmp(&slow_with_tagged_index); + } else { + DCHECK(handler_requirement == kMissOnMissingHandler); + __ jmp(&miss); + } + // Extra capacity case: Check if there is extra capacity to // perform the store and update the length. Used for adding one // element to the array by writing to array[array.length]. @@ -648,6 +665,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, kCheckMap, kDontIncrementLength); KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow, &slow, kDontCheckMap, kIncrementLength); + + if (handler_requirement == kMissOnMissingHandler) { + __ bind(&miss); + GenerateMiss(masm); + } } diff --git a/src/ic/x64/stub-cache-x64.cc b/src/ic/x64/stub-cache-x64.cc index a54ddca..8aff1ea 100644 --- a/src/ic/x64/stub-cache-x64.cc +++ b/src/ic/x64/stub-cache-x64.cc @@ -41,14 +41,14 @@ static void ProbeTable(Isolate* isolate, MacroAssembler* masm, __ LoadAddress(kScratchRegister, key_offset); // Check that the key in the entry matches the name. - // Multiply entry offset by 16 to get the entry address. Since the - // offset register already holds the entry offset times four, multiply - // by a further four. - __ cmpl(name, Operand(kScratchRegister, offset, scale_factor, 0)); + __ cmpp(name, Operand(kScratchRegister, offset, scale_factor, 0)); __ j(not_equal, &miss); // Get the map entry from the cache. // Use key_offset + kPointerSize * 2, rather than loading map_offset. + DCHECK(isolate->stub_cache()->map_reference(table).address() - + isolate->stub_cache()->key_reference(table).address() == + kPointerSize * 2); __ movp(kScratchRegister, Operand(kScratchRegister, offset, scale_factor, kPointerSize * 2)); __ cmpp(kScratchRegister, FieldOperand(receiver, HeapObject::kMapOffset)); -- 2.7.4