From: erik.corry@gmail.com Date: Mon, 6 Aug 2012 14:25:19 +0000 (+0000) Subject: Improve load IC so it can call a native accessor even if the holder is X-Git-Tag: upstream/4.7.83~16185 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=92f30d1df50a7e8ea624c45748070edccdfb9237;p=platform%2Fupstream%2Fv8.git Improve load IC so it can call a native accessor even if the holder is in dictionary mode. Add a flag to all maps to indicate whether they are used for dictionary (normalized) objects or fast mode objects. Review URL: https://chromiumcodereview.appspot.com/10831153 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12264 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index f236441..37a3c4f 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -1230,6 +1230,42 @@ void StubCompiler::GenerateLoadConstant(Handle object, } +void StubCompiler::GenerateDictionaryLoadCallback(Register receiver, + Register name_reg, + Register scratch1, + Register scratch2, + Register scratch3, + Handle callback, + Handle name, + Label* miss) { + Register dictionary = scratch1; + Register index = scratch2; + __ ldr(dictionary, FieldMemOperand(receiver, JSObject::kPropertiesOffset)); + + // Probe the dictionary. + Label probe_done; + StringDictionaryLookupStub::GeneratePositiveLookup(masm(), + miss, + &probe_done, + dictionary, + name_reg, + index, // Set if we hit. + scratch3); + __ bind(&probe_done); + + // If probing finds an entry in the dictionary, check that the value is the + // callback. + const int kElementsStartOffset = + StringDictionary::kHeaderSize + + StringDictionary::kElementsStartIndex * kPointerSize; + const int kValueOffset = kElementsStartOffset + kPointerSize; + __ add(scratch1, dictionary, Operand(kValueOffset - kHeapObjectTag)); + __ ldr(scratch3, MemOperand(scratch1, index, LSL, kPointerSizeLog2)); + __ cmp(scratch3, Operand(callback)); + __ b(ne, miss); +} + + void StubCompiler::GenerateLoadCallback(Handle object, Handle holder, Register receiver, @@ -1247,6 +1283,11 @@ void StubCompiler::GenerateLoadCallback(Handle object, Register reg = CheckPrototypes(object, receiver, holder, scratch1, scratch2, scratch3, name, miss); + if (!holder->HasFastProperties() && !holder->IsJSGlobalObject()) { + GenerateDictionaryLoadCallback( + receiver, name_reg, scratch1, scratch2, scratch3, callback, name, miss); + } + // Build AccessorInfo::args_ list on the stack and push property name below // the exit frame to make GC aware of them and store pointers to them. __ push(receiver); diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index c72af5c..4300e73 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -747,6 +747,7 @@ Handle Genesis::CreateNewGlobals( } js_global_function->initial_map()->set_is_hidden_prototype(); + js_global_function->initial_map()->set_dictionary_map(true); Handle inner_global = factory()->NewGlobalObject(js_global_function); if (inner_global_out != NULL) { @@ -1431,6 +1432,7 @@ bool Genesis::InstallNatives() { Handle name = factory()->LookupAsciiSymbol("builtins"); builtins_fun->shared()->set_instance_class_name(*name); + builtins_fun->initial_map()->set_dictionary_map(true); // Allocate the builtins object. Handle builtins = diff --git a/src/heap.cc b/src/heap.cc index 3fe44f2..9dd91e3 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -2424,6 +2424,7 @@ bool Heap::CreateInitialMaps() { if (!maybe_obj->ToObject(&obj)) return false; } Map* global_context_map = Map::cast(obj); + global_context_map->set_dictionary_map(true); global_context_map->set_visitor_id(StaticVisitorBase::kVisitGlobalContext); set_global_context_map(global_context_map); @@ -4108,6 +4109,7 @@ MaybeObject* Heap::AllocateJSFunctionProxy(Object* handler, MaybeObject* Heap::AllocateGlobalObject(JSFunction* constructor) { ASSERT(constructor->has_initial_map()); Map* map = constructor->initial_map(); + ASSERT(map->is_dictionary_map()); // Make sure no field properties are described in the initial map. // This guarantees us that normalizing the properties does not @@ -4158,6 +4160,7 @@ MaybeObject* Heap::AllocateGlobalObject(JSFunction* constructor) { Map* new_map; MaybeObject* maybe_map = map->CopyDropDescriptors(); if (!maybe_map->To(&new_map)) return maybe_map; + new_map->set_dictionary_map(true); // Set up the global object as a normalized object. global->set_map(new_map); diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 188b53a..f2bf641 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -1743,6 +1743,10 @@ HLoadNamedFieldPolymorphic::HLoadNamedFieldPolymorphic(HValue* context, break; } } else if (lookup.IsCacheable() && + // For dicts the lookup on the map will fail, but the object may + // contain the property so we cannot generate a negative lookup + // (which would just be a map check and return undefined). + !map->is_dictionary_map() && PrototypeChainCanNeverResolve(map, name)) { negative_lookups.Add(types->at(i), zone); } diff --git a/src/hydrogen.cc b/src/hydrogen.cc index e11dbf0..bfbe159 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5215,8 +5215,13 @@ void HGraphBuilder::HandlePropertyAssignment(Assignment* expr) { HInstruction* instr = NULL; SmallMapList* types = expr->GetReceiverTypes(); - if (expr->IsMonomorphic()) { - Handle map = types->first(); + bool monomorphic = expr->IsMonomorphic(); + Handle map; + if (monomorphic) { + map = types->first(); + if (map->is_dictionary_map()) monomorphic = false; + } + if (monomorphic) { Handle accessors; Handle holder; if (LookupAccessorPair(map, name, &accessors, &holder)) { @@ -5392,8 +5397,14 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) { Handle name = prop->key()->AsLiteral()->AsPropertyName(); Handle map; HInstruction* load; - if (prop->IsMonomorphic()) { + bool monomorphic = prop->IsMonomorphic(); + if (monomorphic) { map = prop->GetReceiverTypes()->first(); + // We can't generate code for a monomorphic dict mode load so + // just pretend it is not monomorphic. + if (map->is_dictionary_map()) monomorphic = false; + } + if (monomorphic) { Handle accessors; Handle holder; if (LookupAccessorPair(map, name, &accessors, &holder)) { @@ -5416,7 +5427,7 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) { if (instr->HasObservableSideEffects()) AddSimulate(operation->id()); HInstruction* store; - if (map.is_null()) { + if (!monomorphic) { // If we don't know the monomorphic type, do a generic store. CHECK_ALIVE(store = BuildStoreNamedGeneric(object, name, instr)); } else { @@ -5717,6 +5728,7 @@ HInstruction* HGraphBuilder::BuildLoadNamedMonomorphic(HValue* object, Property* expr, Handle map) { // Handle a load from a known field. + ASSERT(!map->is_dictionary_map()); LookupResult lookup(isolate()); map->LookupDescriptor(NULL, *name, &lookup); if (lookup.IsField()) { @@ -6350,8 +6362,13 @@ void HGraphBuilder::VisitProperty(Property* expr) { Handle name = expr->key()->AsLiteral()->AsPropertyName(); SmallMapList* types = expr->GetReceiverTypes(); + bool monomorphic = expr->IsMonomorphic(); + Handle map; if (expr->IsMonomorphic()) { - Handle map = types->first(); + map = types->first(); + if (map->is_dictionary_map()) monomorphic = false; + } + if (monomorphic) { Handle accessors; Handle holder; if (LookupAccessorPair(map, name, &accessors, &holder)) { @@ -7860,8 +7877,12 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) { Handle name = prop->key()->AsLiteral()->AsPropertyName(); Handle map; HInstruction* load; - if (prop->IsMonomorphic()) { + bool monomorphic = prop->IsMonomorphic(); + if (monomorphic) { map = prop->GetReceiverTypes()->first(); + if (map->is_dictionary_map()) monomorphic = false; + } + if (monomorphic) { Handle accessors; Handle holder; if (LookupAccessorPair(map, name, &accessors, &holder)) { @@ -7879,7 +7900,7 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) { input = Pop(); HInstruction* store; - if (map.is_null()) { + if (!monomorphic) { // If we don't know the monomorphic type, do a generic store. CHECK_ALIVE(store = BuildStoreNamedGeneric(object, name, after)); } else { diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index c1a5c39..9b8a05b 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -1052,6 +1052,42 @@ void StubCompiler::GenerateLoadField(Handle object, } +void StubCompiler::GenerateDictionaryLoadCallback(Register receiver, + Register name_reg, + Register scratch1, + Register scratch2, + Register scratch3, + Handle callback, + Handle name, + Label* miss) { + Register dictionary = scratch1; + __ mov(dictionary, FieldOperand(receiver, JSObject::kPropertiesOffset)); + + // Probe the dictionary. + Label probe_done; + StringDictionaryLookupStub::GeneratePositiveLookup(masm(), + miss, + &probe_done, + dictionary, + name_reg, + scratch2, + scratch3); + __ bind(&probe_done); + + // If probing finds an entry in the dictionary, scratch2 contains the + // index into the dictionary. Check that the value is the callback. + Register index = scratch2; + const int kElementsStartOffset = + StringDictionary::kHeaderSize + + StringDictionary::kElementsStartIndex * kPointerSize; + const int kValueOffset = kElementsStartOffset + kPointerSize; + __ mov(scratch3, + Operand(dictionary, index, times_4, kValueOffset - kHeapObjectTag)); + __ cmp(scratch3, callback); + __ j(not_equal, miss); +} + + void StubCompiler::GenerateLoadCallback(Handle object, Handle holder, Register receiver, @@ -1069,6 +1105,11 @@ void StubCompiler::GenerateLoadCallback(Handle object, Register reg = CheckPrototypes(object, receiver, holder, scratch1, scratch2, scratch3, name, miss); + if (!holder->HasFastProperties() && !holder->IsJSGlobalObject()) { + GenerateDictionaryLoadCallback( + receiver, name_reg, scratch1, scratch2, scratch3, callback, name, miss); + } + // Insert additional parameters into the stack frame above return address. ASSERT(!scratch3.is(reg)); __ pop(scratch3); // Get return address to place it below. diff --git a/src/ic.cc b/src/ic.cc index 6f2677f..43ee602 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -989,7 +989,6 @@ void LoadIC::UpdateCaches(LookupResult* lookup, if (callback->IsAccessorInfo()) { Handle info = Handle::cast(callback); if (v8::ToCData
(info->getter()) == 0) return; - if (!holder->HasFastProperties()) return; if (!info->IsCompatibleReceiver(*receiver)) return; code = isolate()->stub_cache()->ComputeLoadCallback( name, receiver, holder, info); @@ -997,7 +996,6 @@ void LoadIC::UpdateCaches(LookupResult* lookup, Handle getter(Handle::cast(callback)->getter()); if (!getter->IsJSFunction()) return; if (holder->IsGlobalObject()) return; - if (!holder->HasFastProperties()) return; code = isolate()->stub_cache()->ComputeLoadViaGetter( name, receiver, holder, Handle::cast(getter)); } else { diff --git a/src/objects-inl.h b/src/objects-inl.h index 7331101..9bc6b9b 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1608,6 +1608,7 @@ void JSObject::InitializeBody(Map* map, bool JSObject::HasFastProperties() { + ASSERT(properties()->IsDictionary() == map()->is_dictionary_map()); return !properties()->IsDictionary(); } @@ -3007,11 +3008,22 @@ void Map::set_is_shared(bool value) { set_bit_field3(IsShared::update(bit_field3(), value)); } + bool Map::is_shared() { return IsShared::decode(bit_field3()); } +void Map::set_dictionary_map(bool value) { + set_bit_field3(DictionaryMap::update(bit_field3(), value)); +} + + +bool Map::is_dictionary_map() { + return DictionaryMap::decode(bit_field3()); +} + + JSFunction* Map::unchecked_constructor() { return reinterpret_cast(READ_FIELD(this, kConstructorOffset)); } diff --git a/src/objects.cc b/src/objects.cc index b612c7d..749a352 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -516,6 +516,7 @@ MaybeObject* JSObject::DeleteNormalizedProperty(String* name, DeleteMode mode) { MaybeObject* maybe_new_map = map()->CopyDropDescriptors(); if (!maybe_new_map->To(&new_map)) return maybe_new_map; + ASSERT(new_map->is_dictionary_map()); set_map(new_map); } JSGlobalPropertyCell* cell = @@ -3218,6 +3219,7 @@ MaybeObject* NormalizedMapCache::Get(JSObject* obj, fast->CopyNormalized(mode, SHARED_NORMALIZED_MAP); if (!maybe_result->ToObject(&result)) return maybe_result; } + ASSERT(Map::cast(result)->is_dictionary_map()); set(index, result); isolate->counters()->normalized_maps()->Increment(); @@ -3343,6 +3345,7 @@ MaybeObject* JSObject::NormalizeProperties(PropertyNormalizationMode mode, current_heap->isolate()->context()->global_context()-> normalized_map_cache()->Get(this, mode); if (!maybe_map->To(&new_map)) return maybe_map; + ASSERT(new_map->is_dictionary_map()); // We have now successfully allocated all the necessary objects. // Changes can now be made with the guarantee that all of them take effect. @@ -4512,6 +4515,7 @@ MaybeObject* JSObject::SetPropertyCallback(String* name, Map* new_map; MaybeObject* maybe_new_map = map()->CopyDropDescriptors(); if (!maybe_new_map->To(&new_map)) return maybe_new_map; + ASSERT(new_map->is_dictionary_map()); set_map(new_map); // When running crankshaft, changing the map is not enough. We @@ -4873,6 +4877,7 @@ MaybeObject* Map::CopyNormalized(PropertyNormalizationMode mode, result->set_code_cache(code_cache()); result->set_is_shared(sharing == SHARED_NORMALIZED_MAP); + result->set_dictionary_map(true); #ifdef DEBUG if (FLAG_verify_heap && result->is_shared()) { @@ -7330,7 +7335,8 @@ bool Map::EquivalentToForNormalization(Map* other, bit_field2() == other->bit_field2() && static_cast(bit_field3()) == LastAddedBits::update( - IsShared::update(other->bit_field3(), true), + IsShared::update(DictionaryMap::update(other->bit_field3(), true), + true), kNoneAdded); } @@ -12572,6 +12578,7 @@ MaybeObject* StringDictionary::TransformPropertiesToFastFor( Map* new_map; MaybeObject* maybe_new_map = obj->map()->CopyDropDescriptors(); if (!maybe_new_map->To(&new_map)) return maybe_new_map; + new_map->set_dictionary_map(false); if (instance_descriptor_length == 0) { ASSERT_LE(unused_property_fields, inobject_props); diff --git a/src/objects.h b/src/objects.h index 4f85234..1c7a9e3 100644 --- a/src/objects.h +++ b/src/objects.h @@ -4699,7 +4699,8 @@ class Map: public HeapObject { class IsShared: public BitField {}; class FunctionWithPrototype: public BitField {}; - class LastAddedBits: public BitField {}; + class DictionaryMap: public BitField {}; + class LastAddedBits: public BitField {}; // Tells whether the object in the prototype property will be used // for instances created from this function. If the prototype @@ -4845,6 +4846,13 @@ class Map: public HeapObject { inline void set_is_shared(bool value); inline bool is_shared(); + // Tells whether the map is used for JSObjects in dictionary mode (ie + // normalized objects, ie objects for which HasFastProperties returns false). + // A map can never be used for both dictionary mode and fast mode JSObjects. + // False by default and for HeapObjects that are not JSObjects. + inline void set_dictionary_map(bool value); + inline bool is_dictionary_map(); + // Tells whether the instance needs security checks when accessing its // properties. inline void set_is_access_check_needed(bool access_check_needed); @@ -5165,6 +5173,7 @@ class Map: public HeapObject { // Bit positions for bit field 3 static const int kIsShared = 0; static const int kFunctionWithPrototype = 1; + static const int kDictionaryMap = 2; typedef FixedBodyDescriptor name, Label* miss); + void GenerateDictionaryLoadCallback(Register receiver, + Register name_reg, + Register scratch1, + Register scratch2, + Register scratch3, + Handle callback, + Handle name, + Label* miss); + void GenerateLoadConstant(Handle object, Handle holder, Register receiver, diff --git a/src/type-info.cc b/src/type-info.cc index 4d87fc3..a586fa0 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -100,11 +100,12 @@ bool TypeFeedbackOracle::LoadIsMonomorphicNormal(Property* expr) { if (map_or_code->IsMap()) return true; if (map_or_code->IsCode()) { Handle code = Handle::cast(map_or_code); - return code->is_keyed_load_stub() && + bool preliminary_checks = code->is_keyed_load_stub() && code->ic_state() == MONOMORPHIC && - Code::ExtractTypeFromFlags(code->flags()) == Code::NORMAL && - code->FindFirstMap() != NULL && - !CanRetainOtherContext(code->FindFirstMap(), *global_context_); + Code::ExtractTypeFromFlags(code->flags()) == Code::NORMAL; + if (!preliminary_checks) return false; + Map* map = code->FindFirstMap(); + return map != NULL && !CanRetainOtherContext(map, *global_context_); } return false; } @@ -131,12 +132,14 @@ bool TypeFeedbackOracle::StoreIsMonomorphicNormal(TypeFeedbackId ast_id) { bool allow_growth = Code::GetKeyedAccessGrowMode(code->extra_ic_state()) == ALLOW_JSARRAY_GROWTH; - return code->is_keyed_store_stub() && + bool preliminary_checks = + code->is_keyed_store_stub() && !allow_growth && code->ic_state() == MONOMORPHIC && - Code::ExtractTypeFromFlags(code->flags()) == Code::NORMAL && - code->FindFirstMap() != NULL && - !CanRetainOtherContext(code->FindFirstMap(), *global_context_); + Code::ExtractTypeFromFlags(code->flags()) == Code::NORMAL; + if (!preliminary_checks) return false; + Map* map = code->FindFirstMap(); + return map != NULL && !CanRetainOtherContext(map, *global_context_); } return false; } diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index df6b5d5..c2b0b7f 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -1029,6 +1029,43 @@ void StubCompiler::GenerateLoadField(Handle object, } +void StubCompiler::GenerateDictionaryLoadCallback(Register receiver, + Register name_reg, + Register scratch1, + Register scratch2, + Register scratch3, + Handle callback, + Handle name, + Label* miss) { + Register dictionary = scratch1; + __ movq(dictionary, FieldOperand(receiver, JSObject::kPropertiesOffset)); + + // Probe the dictionary. + Label probe_done; + StringDictionaryLookupStub::GeneratePositiveLookup(masm(), + miss, + &probe_done, + dictionary, + name_reg, + scratch2, + scratch3); + __ bind(&probe_done); + + // If probing finds an entry in the dictionary, scratch2 contains the + // index into the dictionary. Check that the value is the callback. + Register index = scratch2; + const int kElementsStartOffset = + StringDictionary::kHeaderSize + + StringDictionary::kElementsStartIndex * kPointerSize; + const int kValueOffset = kElementsStartOffset + kPointerSize; + __ movq(scratch3, + Operand(dictionary, index, times_8, kValueOffset - kHeapObjectTag)); + __ movq(scratch2, callback, RelocInfo::EMBEDDED_OBJECT); + __ cmpq(scratch3, scratch2); + __ j(not_equal, miss); +} + + void StubCompiler::GenerateLoadCallback(Handle object, Handle holder, Register receiver, @@ -1046,6 +1083,11 @@ void StubCompiler::GenerateLoadCallback(Handle object, Register reg = CheckPrototypes(object, receiver, holder, scratch1, scratch2, scratch3, name, miss); + if (!holder->HasFastProperties() && !holder->IsJSGlobalObject()) { + GenerateDictionaryLoadCallback( + receiver, name_reg, scratch1, scratch2, scratch3, callback, name, miss); + } + // Insert additional parameters into the stack frame above return address. ASSERT(!scratch2.is(reg)); __ pop(scratch2); // Get return address to place it below. diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 77c0952..1aad006 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -16973,9 +16973,9 @@ TEST(TryFinallyMessage) { } -THREADED_TEST(Regress137002a) { - i::FLAG_allow_natives_syntax = true; - v8::HandleScope scope; +static void Helper137002(bool do_store, + bool polymorphic, + bool remove_accessor) { LocalContext context; Local templ = ObjectTemplate::New(); templ->SetAccessor(v8_str("foo"), @@ -16985,13 +16985,46 @@ THREADED_TEST(Regress137002a) { // Turn monomorphic on slow object with native accessor, then turn // polymorphic, finally optimize to create negative lookup and fail. - CompileRun("function f(x) { return x.foo; }" + CompileRun(do_store ? + "function f(x) { x.foo = void 0; }" : + "function f(x) { return x.foo; }"); + CompileRun("obj.y = void 0;" "%OptimizeObjectForAddingMultipleProperties(obj, 1);" "obj.__proto__ = null;" - "f(obj); f(obj); f({});" - "%OptimizeFunctionOnNextCall(f);" - "var result = f(obj);"); - CHECK_EQ(42, context->Global()->Get(v8_str("result"))->Int32Value()); + "f(obj); f(obj);"); + if (polymorphic) { + CompileRun("f({});"); + } + CompileRun("obj.y = void 0;" + "%OptimizeFunctionOnNextCall(f);"); + if (remove_accessor) { + CompileRun("delete obj.foo;"); + } + CompileRun("var result = f(obj);"); + if (do_store) { + CompileRun("result = obj.y;"); + } + if (remove_accessor) { + CHECK(context->Global()->Get(v8_str("result"))->IsUndefined()); + } else { + CHECK_EQ(do_store ? 23 : 42, + context->Global()->Get(v8_str("result"))->Int32Value()); + } +} + + +THREADED_TEST(Regress137002a) { + i::FLAG_allow_natives_syntax = true; + i::FLAG_compilation_cache = false; + v8::HandleScope scope; + Helper137002(false, false, false); + Helper137002(false, false, true); + Helper137002(false, true, false); + Helper137002(false, true, true); + Helper137002(true, false, false); + Helper137002(true, false, true); + Helper137002(true, true, false); + Helper137002(true, true, true); }