From 0e9a480fa59ebaddc456091520a6325b753181e3 Mon Sep 17 00:00:00 2001 From: Igor Sheludko Date: Mon, 10 May 2021 14:07:41 +0200 Subject: [PATCH] [ic] Clarify lookup start object vs holder more MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL also allows reusing slow case for API callbacks. Bug: chromium:1201781 Change-Id: Ib5f81c510404060c888ba30c82357d6ed1a95cf5 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2882809 Commit-Queue: Igor Sheludko Reviewed-by: Marja Hölttä Cr-Commit-Position: refs/heads/master@{#74470} --- src/ic/accessor-assembler.cc | 5 +-- src/ic/ic.cc | 15 ++++---- src/runtime/runtime-object.cc | 65 ++++++++++++++++++----------------- src/runtime/runtime.h | 5 +-- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/ic/accessor-assembler.cc b/src/ic/accessor-assembler.cc index 463d379367..bada0370f3 100644 --- a/src/ic/accessor-assembler.cc +++ b/src/ic/accessor-assembler.cc @@ -696,8 +696,9 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase( p->name(), p->slot(), p->vector()); } else { - exit_point->ReturnCallRuntime(Runtime::kGetProperty, p->context(), holder, - p->name(), p->receiver()); + exit_point->ReturnCallRuntime(Runtime::kGetProperty, p->context(), + p->lookup_start_object(), p->name(), + p->receiver()); } } diff --git a/src/ic/ic.cc b/src/ic/ic.cc index b7723559c3..5031c8a7f3 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -774,23 +774,20 @@ void IC::SetCache(Handle name, const MaybeObjectHandle& handler) { } void LoadIC::UpdateCaches(LookupIterator* lookup) { - Handle code; + Handle handler; if (lookup->state() == LookupIterator::ACCESS_CHECK) { - code = LoadHandler::LoadSlow(isolate()); + handler = LoadHandler::LoadSlow(isolate()); } else if (!lookup->IsFound()) { TRACE_HANDLER_STATS(isolate(), LoadIC_LoadNonexistentDH); Handle smi_handler = LoadHandler::LoadNonExistent(isolate()); - code = LoadHandler::LoadFullChain( + handler = LoadHandler::LoadFullChain( isolate(), lookup_start_object_map(), MaybeObjectHandle(isolate()->factory()->null_value()), smi_handler); } else if (IsLoadGlobalIC() && lookup->state() == LookupIterator::JSPROXY) { // If there is proxy just install the slow stub since we need to call the // HasProperty trap for global loads. The ProxyGetProperty builtin doesn't // handle this case. - Handle slow_handler = LoadHandler::LoadSlow(isolate()); - Handle holder = lookup->GetHolder(); - code = LoadHandler::LoadFromPrototype(isolate(), lookup_start_object_map(), - holder, slow_handler); + handler = LoadHandler::LoadSlow(isolate()); } else { if (IsLoadGlobalIC()) { if (lookup->TryLookupCachedProperty()) { @@ -805,12 +802,12 @@ void LoadIC::UpdateCaches(LookupIterator* lookup) { return; } } - code = ComputeHandler(lookup); + handler = ComputeHandler(lookup); } // Can't use {lookup->name()} because the LookupIterator might be in // "elements" mode for keys that are strings representing integers above // JSArray::kMaxIndex. - SetCache(lookup->GetName(), code); + SetCache(lookup->GetName(), handler); TraceIC("LoadIC", lookup->GetName()); } diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index eec942088e..23ce3d8e75 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -27,23 +27,22 @@ namespace v8 { namespace internal { -MaybeHandle Runtime::GetObjectProperty(Isolate* isolate, - Handle holder, - Handle key, - Handle receiver, - bool* is_found) { +MaybeHandle Runtime::GetObjectProperty( + Isolate* isolate, Handle lookup_start_object, Handle key, + Handle receiver, bool* is_found) { if (receiver.is_null()) { - receiver = holder; + receiver = lookup_start_object; } - if (holder->IsNullOrUndefined(isolate)) { - ErrorUtils::ThrowLoadFromNullOrUndefined(isolate, holder, key); + if (lookup_start_object->IsNullOrUndefined(isolate)) { + ErrorUtils::ThrowLoadFromNullOrUndefined(isolate, lookup_start_object, key); return MaybeHandle(); } bool success = false; LookupIterator::Key lookup_key(isolate, key, &success); if (!success) return MaybeHandle(); - LookupIterator it = LookupIterator(isolate, receiver, lookup_key, holder); + LookupIterator it = + LookupIterator(isolate, receiver, lookup_key, lookup_start_object); MaybeHandle result = Object::GetProperty(&it); if (is_found) *is_found = it.IsFound(); @@ -60,12 +59,12 @@ MaybeHandle Runtime::GetObjectProperty(Isolate* isolate, : name_string; THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kInvalidPrivateBrand, - class_name, holder), + class_name, lookup_start_object), Object); } THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kInvalidPrivateMemberRead, - name_string, holder), + name_string, lookup_start_object), Object); } return result; @@ -707,15 +706,15 @@ RUNTIME_FUNCTION(Runtime_JSReceiverSetPrototypeOfDontThrow) { RUNTIME_FUNCTION(Runtime_GetProperty) { HandleScope scope(isolate); DCHECK(args.length() == 3 || args.length() == 2); - CONVERT_ARG_HANDLE_CHECKED(Object, holder_obj, 0); + CONVERT_ARG_HANDLE_CHECKED(Object, lookup_start_obj, 0); CONVERT_ARG_HANDLE_CHECKED(Object, key_obj, 1); - Handle receiver_obj = holder_obj; + Handle receiver_obj = lookup_start_obj; if (args.length() == 3) { CHECK(args[2].IsObject()); receiver_obj = args.at(2); } - // Fast cases for getting named properties of the holder JSObject + // Fast cases for getting named properties of the lookup_start_obj JSObject // itself. // // The global proxy objects has to be excluded since LookupOwn on @@ -733,18 +732,19 @@ RUNTIME_FUNCTION(Runtime_GetProperty) { if (key_obj->IsString() && String::cast(*key_obj).AsArrayIndex(&index)) { key_obj = isolate->factory()->NewNumberFromUint(index); } - if (holder_obj->IsJSObject()) { - if (!holder_obj->IsJSGlobalProxy() && !holder_obj->IsAccessCheckNeeded() && - key_obj->IsName()) { - Handle holder = Handle::cast(holder_obj); + if (lookup_start_obj->IsJSObject()) { + Handle lookup_start_object = + Handle::cast(lookup_start_obj); + if (!lookup_start_object->IsJSGlobalProxy() && + !lookup_start_object->IsAccessCheckNeeded() && key_obj->IsName()) { Handle key = Handle::cast(key_obj); key_obj = key = isolate->factory()->InternalizeName(key); DisallowGarbageCollection no_gc; - if (holder->IsJSGlobalObject()) { + if (lookup_start_object->IsJSGlobalObject()) { // Attempt dictionary lookup. - GlobalDictionary dictionary = - JSGlobalObject::cast(*holder).global_dictionary(kAcquireLoad); + GlobalDictionary dictionary = JSGlobalObject::cast(*lookup_start_object) + .global_dictionary(kAcquireLoad); InternalIndex entry = dictionary.FindEntry(isolate, key); if (entry.is_found()) { PropertyCell cell = dictionary.CellAt(entry); @@ -754,17 +754,19 @@ RUNTIME_FUNCTION(Runtime_GetProperty) { // If value is the hole (meaning, absent) do the general lookup. } } - } else if (!holder->HasFastProperties()) { + } else if (!lookup_start_object->HasFastProperties()) { // Attempt dictionary lookup. if (V8_ENABLE_SWISS_NAME_DICTIONARY_BOOL) { - SwissNameDictionary dictionary = holder->property_dictionary_swiss(); + SwissNameDictionary dictionary = + lookup_start_object->property_dictionary_swiss(); InternalIndex entry = dictionary.FindEntry(isolate, *key); if (entry.is_found() && (dictionary.DetailsAt(entry).kind() == kData)) { return dictionary.ValueAt(entry); } } else { - NameDictionary dictionary = holder->property_dictionary(); + NameDictionary dictionary = + lookup_start_object->property_dictionary(); InternalIndex entry = dictionary.FindEntry(isolate, key); if ((entry.is_found()) && (dictionary.DetailsAt(entry).kind() == kData)) { @@ -779,22 +781,21 @@ RUNTIME_FUNCTION(Runtime_GetProperty) { // transition elements to FAST_*_ELEMENTS to avoid excessive boxing of // doubles for those future calls in the case that the elements would // become PACKED_DOUBLE_ELEMENTS. - Handle js_object = Handle::cast(holder_obj); - ElementsKind elements_kind = js_object->GetElementsKind(); + ElementsKind elements_kind = lookup_start_object->GetElementsKind(); if (IsDoubleElementsKind(elements_kind)) { - if (Smi::ToInt(*key_obj) >= js_object->elements().length()) { + if (Smi::ToInt(*key_obj) >= lookup_start_object->elements().length()) { elements_kind = IsHoleyElementsKind(elements_kind) ? HOLEY_ELEMENTS : PACKED_ELEMENTS; - JSObject::TransitionElementsKind(js_object, elements_kind); + JSObject::TransitionElementsKind(lookup_start_object, elements_kind); } } else { DCHECK(IsSmiOrObjectElementsKind(elements_kind) || !IsFastElementsKind(elements_kind)); } } - } else if (holder_obj->IsString() && key_obj->IsSmi()) { + } else if (lookup_start_obj->IsString() && key_obj->IsSmi()) { // Fast case for string indexing using [] with a smi index. - Handle str = Handle::cast(holder_obj); + Handle str = Handle::cast(lookup_start_obj); int index = Handle::cast(key_obj)->value(); if (index >= 0 && index < str->length()) { Factory* factory = isolate->factory(); @@ -805,8 +806,8 @@ RUNTIME_FUNCTION(Runtime_GetProperty) { // Fall back to GetObjectProperty. RETURN_RESULT_OR_FAILURE( - isolate, - Runtime::GetObjectProperty(isolate, holder_obj, key_obj, receiver_obj)); + isolate, Runtime::GetObjectProperty(isolate, lookup_start_obj, key_obj, + receiver_obj)); } RUNTIME_FUNCTION(Runtime_SetKeyedProperty) { diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index aa6ab53b0f..13ad9c7c05 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -788,9 +788,10 @@ class Runtime : public AllStatic { Handle value, StoreOrigin store_origin, Maybe should_throw = Nothing()); - // When "receiver" is not passed, it defaults to "holder". + // When "receiver" is not passed, it defaults to "lookup_start_object". V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle - GetObjectProperty(Isolate* isolate, Handle holder, Handle key, + GetObjectProperty(Isolate* isolate, Handle lookup_start_object, + Handle key, Handle receiver = Handle(), bool* is_found = nullptr);