From 69a7e86a5eadba764826b1464a557b3f8b669e2d Mon Sep 17 00:00:00 2001 From: Maya Lekova Date: Tue, 20 Aug 2019 11:42:33 +0200 Subject: [PATCH] [turbofan] Fully brokerize ReducePropertyAccess Bug: v8:7790 Change-Id: I6f493d994f49d84020966322d60061567b54c854 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1760808 Commit-Queue: Maya Lekova Reviewed-by: Georg Neis Cr-Commit-Position: refs/heads/master@{#63265} --- src/compiler/heap-refs.h | 4 ++ src/compiler/js-heap-broker.cc | 50 ++++++++++++++++++- .../js-native-context-specialization.cc | 33 ++++++------ .../js-native-context-specialization.h | 2 +- .../serializer-for-background-compilation.cc | 37 ++++++++++---- 5 files changed, 98 insertions(+), 28 deletions(-) diff --git a/src/compiler/heap-refs.h b/src/compiler/heap-refs.h index c52726f7fe..a3562da6f1 100644 --- a/src/compiler/heap-refs.h +++ b/src/compiler/heap-refs.h @@ -592,6 +592,7 @@ class V8_EXPORT_PRIVATE MapRef : public HeapObjectRef { bool supports_fast_array_iteration() const; bool supports_fast_array_resize() const; bool IsMapOfCurrentGlobalProxy() const; + bool is_abandoned_prototype_map() const; OddballType oddball_type() const; @@ -624,6 +625,9 @@ class V8_EXPORT_PRIVATE MapRef : public HeapObjectRef { bool IsUnboxedDoubleField(int descriptor_index) const; ObjectRef GetStrongValue(int descriptor_number) const; + void SerializeRootMap(); + base::Optional FindRootMap() const; + // Available after calling JSFunctionRef::Serialize on a function that has // this map as initial map. ObjectRef GetConstructor() const; diff --git a/src/compiler/js-heap-broker.cc b/src/compiler/js-heap-broker.cc index c89e0580f2..00bfbbb7d4 100644 --- a/src/compiler/js-heap-broker.cc +++ b/src/compiler/js-heap-broker.cc @@ -974,6 +974,9 @@ class MapData : public HeapObjectData { bool IsMapOfCurrentGlobalProxy() const { return is_map_of_current_global_proxy_; } + bool is_abandoned_prototype_map() const { + return is_abandoned_prototype_map_; + } // Extra information. @@ -992,6 +995,9 @@ class MapData : public HeapObjectData { return instance_descriptors_; } + void SerializeRootMap(JSHeapBroker* broker); + MapData* FindRootMap() const; + void SerializeConstructor(JSHeapBroker* broker); ObjectData* GetConstructor() const { CHECK(serialized_constructor_); @@ -1031,6 +1037,7 @@ class MapData : public HeapObjectData { bool const supports_fast_array_iteration_; bool const supports_fast_array_resize_; bool const is_map_of_current_global_proxy_; + bool const is_abandoned_prototype_map_; bool serialized_elements_kind_generalizations_ = false; ZoneVector elements_kind_generalizations_; @@ -1047,6 +1054,9 @@ class MapData : public HeapObjectData { bool serialized_prototype_ = false; ObjectData* prototype_ = nullptr; + bool serialized_root_map_ = false; + MapData* root_map_ = nullptr; + bool serialized_for_element_load_ = false; bool serialized_for_element_store_ = false; @@ -1155,6 +1165,7 @@ MapData::MapData(JSHeapBroker* broker, ObjectData** storage, Handle object) SupportsFastArrayResize(broker->isolate(), object)), is_map_of_current_global_proxy_( object->IsMapOfGlobalProxy(broker->isolate()->native_context())), + is_abandoned_prototype_map_(object->is_abandoned_prototype_map()), elements_kind_generalizations_(broker->zone()) {} JSFunctionData::JSFunctionData(JSHeapBroker* broker, ObjectData** storage, @@ -2009,6 +2020,19 @@ void MapData::SerializeOwnDescriptor(JSHeapBroker* broker, << contents.size() << " total)"); } +void MapData::SerializeRootMap(JSHeapBroker* broker) { + if (serialized_root_map_) return; + serialized_root_map_ = true; + + TraceScope tracer(broker, this, "MapData::SerializeRootMap"); + Handle map = Handle::cast(object()); + DCHECK_NULL(root_map_); + root_map_ = + broker->GetOrCreateData(map->FindRootMap(broker->isolate()))->AsMap(); +} + +MapData* MapData::FindRootMap() const { return root_map_; } + void JSObjectData::SerializeRecursiveAsBoilerplate(JSHeapBroker* broker, int depth) { if (serialized_as_boilerplate_) return; @@ -3091,6 +3115,7 @@ BIMODAL_ACCESSOR(Map, HeapObject, prototype) BIMODAL_ACCESSOR_C(Map, InstanceType, instance_type) BIMODAL_ACCESSOR(Map, Object, GetConstructor) BIMODAL_ACCESSOR(Map, HeapObject, GetBackPointer) +BIMODAL_ACCESSOR_C(Map, bool, is_abandoned_prototype_map) #define DEF_NATIVE_CONTEXT_ACCESSOR(type, name) \ BIMODAL_ACCESSOR(NativeContext, type, name) @@ -3228,6 +3253,26 @@ ObjectRef MapRef::GetStrongValue(int descriptor_index) const { return ObjectRef(broker(), data()->AsMap()->GetStrongValue(descriptor_index)); } +void MapRef::SerializeRootMap() { + if (broker()->mode() == JSHeapBroker::kDisabled) return; + CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing); + data()->AsMap()->SerializeRootMap(broker()); +} + +base::Optional MapRef::FindRootMap() const { + if (broker()->mode() == JSHeapBroker::kDisabled) { + AllowHandleDereference allow_handle_dereference; + return MapRef(broker(), handle(object()->FindRootMap(broker()->isolate()), + broker()->isolate())); + } + MapData* map_data = data()->AsMap()->FindRootMap(); + if (map_data) { + return MapRef(broker(), map_data); + } + TRACE_BROKER_MISSING(broker(), "root map for object " << object()); + return base::nullopt; +} + void* JSTypedArrayRef::external_pointer() const { if (broker()->mode() == JSHeapBroker::kDisabled) { AllowHandleDereference allow_handle_dereference; @@ -4401,7 +4446,10 @@ ElementAccessFeedback const& JSHeapBroker::ProcessFeedbackMapsForElementAccess( MapHandles possible_transition_targets; possible_transition_targets.reserve(maps.size()); for (Handle map : maps) { - if (CanInlineElementAccess(MapRef(this, map)) && + MapRef map_ref(this, map); + map_ref.SerializeRootMap(); + + if (CanInlineElementAccess(map_ref) && IsFastElementsKind(map->elements_kind()) && GetInitialFastElementsKind() != map->elements_kind()) { possible_transition_targets.push_back(map); diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index f8274f48c2..25484ea057 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc @@ -1481,18 +1481,16 @@ base::Optional GetTypedArrayConstant(JSHeapBroker* broker, void JSNativeContextSpecialization::RemoveImpossibleReceiverMaps( Node* receiver, ZoneVector>* receiver_maps) const { - // TODO(neis): Decide what to do about this for concurrent inlining. - AllowHandleAllocation allow_handle_allocation; - AllowHandleDereference allow_handle_dereference; - Handle root_map; - if (InferReceiverRootMap(receiver).ToHandle(&root_map)) { + base::Optional root_map = InferReceiverRootMap(receiver); + if (root_map.has_value()) { DCHECK(!root_map->is_abandoned_prototype_map()); - Isolate* isolate = this->isolate(); receiver_maps->erase( std::remove_if(receiver_maps->begin(), receiver_maps->end(), - [root_map, isolate](Handle map) { - return map->is_abandoned_prototype_map() || - map->FindRootMap(isolate) != *root_map; + [root_map, this](Handle map) { + MapRef map_ref(broker(), map); + return map_ref.is_abandoned_prototype_map() || + (map_ref.FindRootMap().has_value() && + !map_ref.FindRootMap()->equals(*root_map)); }), receiver_maps->end()); } @@ -1826,6 +1824,8 @@ Reduction JSNativeContextSpecialization::ReduceElementLoadFromHeapConstant( Reduction JSNativeContextSpecialization::ReducePropertyAccess( Node* node, Node* key, base::Optional static_name, Node* value, FeedbackSource const& source, AccessMode access_mode) { + DisallowHeapAccessIf disallow_heap_access(FLAG_concurrent_inlining); + DCHECK_EQ(key == nullptr, static_name.has_value()); DCHECK(node->opcode() == IrOpcode::kJSLoadProperty || node->opcode() == IrOpcode::kJSStoreProperty || @@ -3230,21 +3230,24 @@ bool JSNativeContextSpecialization::InferReceiverMaps( return false; } -MaybeHandle JSNativeContextSpecialization::InferReceiverRootMap( +base::Optional JSNativeContextSpecialization::InferReceiverRootMap( Node* receiver) const { HeapObjectMatcher m(receiver); if (m.HasValue()) { - return handle(m.Value()->map().FindRootMap(isolate()), isolate()); + MapRef map = m.Ref(broker()).map(); + return map.FindRootMap(); } else if (m.IsJSCreate()) { base::Optional initial_map = NodeProperties::GetJSCreateMap(broker(), receiver); if (initial_map.has_value()) { - DCHECK_EQ(*initial_map->object(), - initial_map->object()->FindRootMap(isolate())); - return initial_map->object(); + if (!initial_map->FindRootMap().has_value()) { + return base::nullopt; + } + DCHECK(initial_map->equals(*initial_map->FindRootMap())); + return *initial_map; } } - return MaybeHandle(); + return base::nullopt; } Graph* JSNativeContextSpecialization::graph() const { diff --git a/src/compiler/js-native-context-specialization.h b/src/compiler/js-native-context-specialization.h index 7898fa96cf..7d0aa5aa34 100644 --- a/src/compiler/js-native-context-specialization.h +++ b/src/compiler/js-native-context-specialization.h @@ -231,7 +231,7 @@ class V8_EXPORT_PRIVATE JSNativeContextSpecialization final // Try to infer a root map for the {receiver} independent of the current // program location. - MaybeHandle InferReceiverRootMap(Node* receiver) const; + base::Optional InferReceiverRootMap(Node* receiver) const; // Checks if we know at compile time that the {receiver} either definitely // has the {prototype} in it's prototype chain, or the {receiver} definitely diff --git a/src/compiler/serializer-for-background-compilation.cc b/src/compiler/serializer-for-background-compilation.cc index 834b801e95..4dd86f321e 100644 --- a/src/compiler/serializer-for-background-compilation.cc +++ b/src/compiler/serializer-for-background-compilation.cc @@ -424,7 +424,7 @@ class SerializerForBackgroundCompilation { bool honor_bailout_on_uninitialized); PropertyAccessInfo ProcessMapForNamedPropertyAccess( - MapRef const& receiver_map, NameRef const& name, AccessMode access_mode, + MapRef receiver_map, NameRef const& name, AccessMode access_mode, base::Optional receiver, Hints* new_accumulator_hints); void ProcessCreateContext(interpreter::BytecodeArrayIterator* iterator, @@ -2245,8 +2245,11 @@ void SerializerForBackgroundCompilation::ProcessUnaryOrBinaryOperation( PropertyAccessInfo SerializerForBackgroundCompilation::ProcessMapForNamedPropertyAccess( - MapRef const& receiver_map, NameRef const& name, AccessMode access_mode, + MapRef receiver_map, NameRef const& name, AccessMode access_mode, base::Optional receiver, Hints* new_accumulator_hints) { + // For JSNativeContextSpecialization::InferReceiverRootMap + receiver_map.SerializeRootMap(); + // For JSNativeContextSpecialization::ReduceNamedAccess. if (receiver_map.IsMapOfCurrentGlobalProxy()) { broker()->native_context().global_proxy_object().GetPropertyCell( @@ -2391,16 +2394,16 @@ void SerializerForBackgroundCompilation::ProcessNamedAccess( Hints receiver, NamedAccessFeedback const& feedback, AccessMode access_mode, Hints* new_accumulator_hints) { for (Handle map : feedback.AsNamedAccess().maps()) { - ProcessMapForNamedPropertyAccess(MapRef(broker(), map), feedback.name(), - access_mode, base::nullopt, - new_accumulator_hints); + MapRef map_ref(broker(), map); + ProcessMapForNamedPropertyAccess(map_ref, feedback.name(), access_mode, + base::nullopt, new_accumulator_hints); } for (Handle map : GetRelevantReceiverMaps(broker()->isolate(), receiver.maps())) { - ProcessMapForNamedPropertyAccess(MapRef(broker(), map), feedback.name(), - access_mode, base::nullopt, - new_accumulator_hints); + MapRef map_ref(broker(), map); + ProcessMapForNamedPropertyAccess(map_ref, feedback.name(), access_mode, + base::nullopt, new_accumulator_hints); } JSGlobalProxyRef global_proxy = @@ -2408,9 +2411,10 @@ void SerializerForBackgroundCompilation::ProcessNamedAccess( for (Handle hint : receiver.constants()) { ObjectRef object(broker(), hint); if (access_mode == AccessMode::kLoad && object.IsJSObject()) { - ProcessMapForNamedPropertyAccess( - object.AsJSObject().map(), feedback.name(), access_mode, - object.AsJSObject(), new_accumulator_hints); + MapRef map_ref = object.AsJSObject().map(); + ProcessMapForNamedPropertyAccess(map_ref, feedback.name(), access_mode, + object.AsJSObject(), + new_accumulator_hints); } // For JSNativeContextSpecialization::ReduceNamedAccessFromNexus. // TODO(neis): This should be done even if megamorphic. @@ -2451,6 +2455,11 @@ void SerializerForBackgroundCompilation::ProcessElementAccess( for (Handle hint : receiver.constants()) { ObjectRef receiver_ref(broker(), hint); + // For JSNativeContextSpecialization::InferReceiverRootMap + if (receiver_ref.IsHeapObject()) { + receiver_ref.AsHeapObject().map().SerializeRootMap(); + } + // For JSNativeContextSpecialization::ReduceElementAccess. if (receiver_ref.IsJSTypedArray()) { receiver_ref.AsJSTypedArray().Serialize(); @@ -2476,6 +2485,12 @@ void SerializerForBackgroundCompilation::ProcessElementAccess( } } } + + // For JSNativeContextSpecialization::InferReceiverRootMap + for (Handle map : receiver.maps()) { + MapRef map_ref(broker(), map); + map_ref.SerializeRootMap(); + } } void SerializerForBackgroundCompilation::VisitLdaNamedProperty(