diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc index 80738e2566..29241bb5c0 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/src/compiler/bytecode-graph-builder.cc @@ -1048,7 +1048,7 @@ BytecodeGraphBuilder::BytecodeGraphBuilder( native_context_(native_context), shared_info_(shared_info), feedback_cell_(feedback_cell), - feedback_vector_(feedback_cell.value().AsFeedbackVector()), + feedback_vector_(feedback_cell.value()->AsFeedbackVector()), invocation_frequency_(invocation_frequency), type_hint_lowering_( broker, jsgraph, feedback_vector_, @@ -4561,7 +4561,8 @@ void BuildGraphFromBytecode(JSHeapBroker* broker, Zone* local_zone, BytecodeGraphBuilderFlags flags, TickCounter* tick_counter) { DCHECK(broker->IsSerializedForCompilation( - shared_info, feedback_cell.value().AsFeedbackVector())); + shared_info, feedback_cell.value()->AsFeedbackVector())); + DCHECK(feedback_cell.value()->AsFeedbackVector().serialized()); BytecodeGraphBuilder builder( broker, local_zone, broker->target_native_context(), shared_info, feedback_cell, osr_offset, jsgraph, invocation_frequency, diff --git a/src/compiler/heap-refs.h b/src/compiler/heap-refs.h index 2ae13dada7..d3abef1582 100644 --- a/src/compiler/heap-refs.h +++ b/src/compiler/heap-refs.h @@ -72,6 +72,7 @@ enum class OddballType : uint8_t { V(ArrayBoilerplateDescription) \ V(CallHandlerInfo) \ V(Cell) \ + V(FeedbackCell) \ V(SharedFunctionInfo) \ V(TemplateObjectDescription) @@ -116,7 +117,6 @@ enum class OddballType : uint8_t { V(AllocationSite) \ V(Code) \ V(DescriptorArray) \ - V(FeedbackCell) \ V(FeedbackVector) \ V(FixedArrayBase) \ V(FunctionTemplateInfo) \ @@ -544,7 +544,7 @@ class FeedbackCellRef : public HeapObjectRef { Handle object() const; base::Optional shared_function_info() const; - HeapObjectRef value() const; + base::Optional value() const; }; class FeedbackVectorRef : public HeapObjectRef { diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index f0708aefc5..0680d19069 100644 --- a/src/compiler/js-call-reducer.cc +++ b/src/compiler/js-call-reducer.cc @@ -4073,8 +4073,13 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) { return ReduceJSCall(node, SharedFunctionInfoRef(broker(), p.shared_info())); } else if (target->opcode() == IrOpcode::kCheckClosure) { FeedbackCellRef cell(broker(), FeedbackCellOf(target->op())); - return ReduceJSCall(node, - cell.value().AsFeedbackVector().shared_function_info()); + if (cell.shared_function_info().has_value()) { + return ReduceJSCall(node, *cell.shared_function_info()); + } else { + TRACE_BROKER_MISSING(broker(), "Unable to reduce JSCall. FeedbackCell " + << cell << " has no FeedbackVector"); + return NoChange(); + } } // If {target} is the result of a JSCreateBoundFunction operation, @@ -4153,11 +4158,10 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) { } else if (feedback_target.has_value() && feedback_target->IsFeedbackCell()) { FeedbackCellRef feedback_cell( broker(), feedback_target.value().AsFeedbackCell().object()); - if (feedback_cell.value().IsFeedbackVector()) { + if (feedback_cell.value().has_value()) { // Check that {target} is a closure with given {feedback_cell}, // which uniquely identifies a given function inside a native context. - FeedbackVectorRef feedback_vector = - feedback_cell.value().AsFeedbackVector(); + FeedbackVectorRef feedback_vector = *feedback_cell.value(); if (!feedback_vector.serialized()) { TRACE_BROKER_MISSING( broker(), "feedback vector, not serialized: " << feedback_vector); diff --git a/src/compiler/js-heap-broker.cc b/src/compiler/js-heap-broker.cc index 3aad32b8d4..00783e3331 100644 --- a/src/compiler/js-heap-broker.cc +++ b/src/compiler/js-heap-broker.cc @@ -1451,7 +1451,11 @@ class FeedbackCellData : public HeapObjectData { FeedbackCellData::FeedbackCellData(JSHeapBroker* broker, ObjectData** storage, Handle object) : HeapObjectData(broker, storage, object), - value_(broker->GetOrCreateData(object->value())) {} + value_(object->value().IsFeedbackVector() + ? broker->GetOrCreateData(object->value()) + : nullptr) { + DCHECK(!FLAG_turbo_direct_heap_access); +} class FeedbackVectorData : public HeapObjectData { public: @@ -2776,62 +2780,93 @@ void JSHeapBroker::InitializeAndStartSerializing( TRACE(this, "Finished serializing standard objects"); } -// clang-format off ObjectData* JSHeapBroker::GetOrCreateData(Handle object, ObjectRef::BackgroundSerialization background_serialization) { - RefsMap::Entry* entry = refs_->LookupOrInsert(object.address()); - ObjectData* object_data = entry->value; + ObjectData* return_value = + TryGetOrCreateData(object, true, background_serialization); + DCHECK_NOT_NULL(return_value); + return return_value; +} - if (object_data == nullptr) { - ObjectData** data_storage = &(entry->value); - // TODO(neis): Remove these Allow* once we serialize everything upfront. - AllowHandleDereference handle_dereference; - if (object->IsSmi()) { - object_data = zone()->New(this, data_storage, object, kSmi); - } else if (IsReadOnlyHeapObject(*object)) { - object_data = zone()->New(this, data_storage, object, - kUnserializedReadOnlyHeapObject); +// clang-format off +ObjectData* JSHeapBroker::TryGetOrCreateData(Handle object, + bool crash_on_error, + ObjectRef::BackgroundSerialization background_serialization) { + RefsMap::Entry* entry = refs_->Lookup(object.address()); + if (entry != nullptr) return entry->value; + + if (mode() == JSHeapBroker::kDisabled) { + entry = refs_->LookupOrInsert(object.address()); + ObjectData** storage = &(entry->value); + if (*storage == nullptr) { + entry->value = zone()->New( + this, storage, object, + object->IsSmi() ? kSmi : kUnserializedHeapObject); + } + return *storage; + } + + ObjectData* object_data; + if (object->IsSmi()) { + entry = refs_->LookupOrInsert(object.address()); + object_data = zone()->New(this, &(entry->value), object, kSmi); + } else if (IsReadOnlyHeapObject(*object)) { + entry = refs_->LookupOrInsert(object.address()); + object_data = zone()->New(this, &(entry->value), object, + kUnserializedReadOnlyHeapObject); // TODO(solanes, v8:10866): Remove the if/else in this macro once we remove the // FLAG_turbo_direct_heap_access. -#define CREATE_DATA_FOR_DIRECT_READ(name) \ - } else if (object->Is##name()) { \ - if (FLAG_turbo_direct_heap_access) { \ - object_data = zone()->New( \ - this, data_storage, object, kNeverSerializedHeapObject); \ - } else { \ - CHECK_EQ(mode(), kSerializing); \ - AllowHandleAllocation handle_allocation; \ - object_data = zone()->New(this, data_storage, \ - Handle::cast(object)); \ - } - HEAP_BROKER_NEVER_SERIALIZED_OBJECT_LIST(CREATE_DATA_FOR_DIRECT_READ) +#define CREATE_DATA_FOR_DIRECT_READ(name) \ + } else if (object->Is##name()) { \ + if (FLAG_turbo_direct_heap_access) { \ + entry = refs_->LookupOrInsert(object.address()); \ + object_data = zone()->New( \ + this, &(entry->value), object, kNeverSerializedHeapObject); \ + } else if (mode() == kSerializing) { \ + AllowHandleAllocation handle_allocation; \ + entry = refs_->LookupOrInsert(object.address()); \ + object_data = zone()->New(this, &(entry->value), \ + Handle::cast(object)); \ + } else { \ + CHECK(!crash_on_error); \ + return nullptr; \ + } + HEAP_BROKER_NEVER_SERIALIZED_OBJECT_LIST(CREATE_DATA_FOR_DIRECT_READ) #undef CREATE_DATA_FOR_DIRECT_READ -#define CREATE_DATA_FOR_POSSIBLE_SERIALIZATION(name) \ - } else if (object->Is##name()) { \ - CHECK_IMPLIES(mode() == kSerialized, \ - background_serialization \ - == ObjectRef::BackgroundSerialization::kAllowed); \ - AllowHandleAllocation handle_allocation; \ - object_data = zone()->New(this, data_storage, \ - Handle::cast(object)); +#define CREATE_DATA_FOR_POSSIBLE_SERIALIZATION(name) \ + } else if (object->Is##name()) { \ + if (mode() == kSerialized && \ + background_serialization != \ + ObjectRef::BackgroundSerialization::kAllowed) { \ + CHECK(!crash_on_error); \ + return nullptr; \ + } \ + AllowHandleAllocation handle_allocation; \ + entry = refs_->LookupOrInsert(object.address()); \ + object_data = zone()->New(this, &(entry->value), \ + Handle::cast(object)); HEAP_BROKER_POSSIBLY_BACKGROUND_SERIALIZED_OBJECT_LIST( CREATE_DATA_FOR_POSSIBLE_SERIALIZATION) #undef CREATE_DATA_FOR_POSSIBLE_SERIALIZATION -#define CREATE_DATA_FOR_SERIALIZATION(name) \ - } else if (object->Is##name()) { \ - CHECK_EQ(mode(), kSerializing); \ - AllowHandleAllocation handle_allocation; \ - object_data = zone()->New(this, data_storage, \ - Handle::cast(object)); - HEAP_BROKER_SERIALIZED_OBJECT_LIST(CREATE_DATA_FOR_SERIALIZATION) -#undef CREATE_DATA_FOR_SERIALIZATION - } else { - UNREACHABLE(); +#define CREATE_DATA_FOR_SERIALIZATION(name) \ + } else if (object->Is##name()) { \ + if (mode() == kSerializing) { \ + entry = refs_->LookupOrInsert(object.address()); \ + AllowHandleAllocation handle_allocation; \ + object_data = zone()->New(this, &(entry->value), \ + Handle::cast(object)); \ + } else { \ + CHECK(!crash_on_error); \ + return nullptr; \ } - // At this point the entry pointer is not guaranteed to be valid as - // the refs_ hash hable could be resized by one of the constructors above. - DCHECK_EQ(object_data, refs_->Lookup(object.address())->value); + HEAP_BROKER_SERIALIZED_OBJECT_LIST(CREATE_DATA_FOR_SERIALIZATION) +#undef CREATE_DATA_FOR_SERIALIZATION + } else { + UNREACHABLE(); } + // At this point the entry pointer is not guaranteed to be valid as + // the refs_ hash hable could be resized by one of the constructors above. + DCHECK_EQ(object_data, refs_->Lookup(object.address())->value); return object_data; } // clang-format on @@ -3583,7 +3618,24 @@ SharedFunctionInfo::Inlineability SharedFunctionInfoRef::GetInlineability() return ObjectRef ::data()->AsSharedFunctionInfo()->GetInlineability(); } -BIMODAL_ACCESSOR(FeedbackCell, HeapObject, value) +base::Optional FeedbackCellRef::value() const { + if (data_->should_access_heap()) { + // Note that we use the synchronized accessor. + Object value = object()->value(kAcquireLoad); + if (!value.IsFeedbackVector()) return base::nullopt; + auto vector_handle = broker()->CanonicalPersistentHandle(value); + ObjectData* vector = broker()->TryGetOrCreateData(vector_handle, false); + if (vector) { + return FeedbackVectorRef(broker(), vector); + } + TRACE_BROKER_MISSING( + broker(), + "Unable to retrieve FeedbackVector from FeedbackCellRef " << *this); + return base::nullopt; + } + ObjectData* vector = ObjectRef::data()->AsFeedbackCell()->value(); + return FeedbackVectorRef(broker(), vector->AsFeedbackVector()); +} base::Optional MapRef::GetStrongValue( InternalIndex descriptor_index) const { @@ -3900,28 +3952,9 @@ ObjectRef::ObjectRef(JSHeapBroker* broker, Handle object, BackgroundSerialization background_serialization, bool check_type) : broker_(broker) { - switch (broker->mode()) { - // We may have to create data in JSHeapBroker::kSerialized as well since we - // read the data from read only heap objects directly instead of serializing - // them. - case JSHeapBroker::kSerialized: - case JSHeapBroker::kSerializing: - data_ = broker->GetOrCreateData(object, background_serialization); - break; - case JSHeapBroker::kDisabled: { - RefsMap::Entry* entry = broker->refs_->LookupOrInsert(object.address()); - ObjectData** storage = &(entry->value); - if (*storage == nullptr) { - entry->value = broker->zone()->New( - broker, storage, object, - object->IsSmi() ? kSmi : kUnserializedHeapObject); - } - data_ = *storage; - break; - } - case JSHeapBroker::kRetired: - UNREACHABLE(); - } + CHECK_NE(broker->mode(), JSHeapBroker::kRetired); + + data_ = broker->GetOrCreateData(object, background_serialization); if (!data_) { // TODO(mslekova): Remove once we're on the background thread. object->Print(); } @@ -4013,10 +4046,10 @@ Float64 FixedDoubleArrayData::Get(int i) const { base::Optional FeedbackCellRef::shared_function_info() const { - if (value().IsFeedbackVector()) { - FeedbackVectorRef vector = value().AsFeedbackVector(); + if (value()) { + FeedbackVectorRef vector = *value(); if (vector.serialized()) { - return value().AsFeedbackVector().shared_function_info(); + return vector.shared_function_info(); } } return base::nullopt; diff --git a/src/compiler/js-heap-broker.h b/src/compiler/js-heap-broker.h index 98a9000d0b..4add819efa 100644 --- a/src/compiler/js-heap-broker.h +++ b/src/compiler/js-heap-broker.h @@ -158,6 +158,13 @@ class V8_EXPORT_PRIVATE JSHeapBroker { Object, ObjectRef::BackgroundSerialization background_serialization = ObjectRef::BackgroundSerialization::kDisallowed); + // Gets data only if we have it. However, thin wrappers will be created for + // smis, read-only objects and never-serialized objects. + ObjectData* TryGetOrCreateData( + Handle, bool crash_on_error, + ObjectRef::BackgroundSerialization background_serialization = + ObjectRef::BackgroundSerialization::kDisallowed); + // Check if {object} is any native context's %ArrayPrototype% or // %ObjectPrototype%. bool IsArrayOrObjectPrototype(const JSObjectRef& object) const; diff --git a/src/compiler/js-heap-copy-reducer.cc b/src/compiler/js-heap-copy-reducer.cc index 7ebc383ea5..3b45b9d82b 100644 --- a/src/compiler/js-heap-copy-reducer.cc +++ b/src/compiler/js-heap-copy-reducer.cc @@ -29,8 +29,10 @@ Reduction JSHeapCopyReducer::Reduce(Node* node) { switch (node->opcode()) { case IrOpcode::kCheckClosure: { FeedbackCellRef cell(broker(), FeedbackCellOf(node->op())); - FeedbackVectorRef feedback_vector = cell.value().AsFeedbackVector(); - feedback_vector.Serialize(); + base::Optional feedback_vector = cell.value(); + if (feedback_vector.has_value()) { + feedback_vector->Serialize(); + } break; } case IrOpcode::kHeapConstant: { diff --git a/src/compiler/js-inlining-heuristic.cc b/src/compiler/js-inlining-heuristic.cc index 41f29da3a3..5d0d9e8868 100644 --- a/src/compiler/js-inlining-heuristic.cc +++ b/src/compiler/js-inlining-heuristic.cc @@ -111,12 +111,9 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions( if (m.IsCheckClosure()) { DCHECK(!out.functions[0].has_value()); FeedbackCellRef feedback_cell(broker(), FeedbackCellOf(m.op())); - SharedFunctionInfoRef shared_info = - feedback_cell.shared_function_info().value(); + SharedFunctionInfoRef shared_info = *feedback_cell.shared_function_info(); out.shared_info = shared_info; - if (feedback_cell.value().IsFeedbackVector() && - CanConsiderForInlining(broker(), shared_info, - feedback_cell.value().AsFeedbackVector())) { + if (CanConsiderForInlining(broker(), shared_info, *feedback_cell.value())) { out.bytecode[0] = shared_info.GetBytecodeArray(); } out.num_functions = 1; @@ -129,9 +126,8 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions( FeedbackCellRef feedback_cell = n.GetFeedbackCellRefChecked(broker()); SharedFunctionInfoRef shared_info(broker(), p.shared_info()); out.shared_info = shared_info; - if (feedback_cell.value().IsFeedbackVector() && - CanConsiderForInlining(broker(), shared_info, - feedback_cell.value().AsFeedbackVector())) { + if (feedback_cell.value().has_value() && + CanConsiderForInlining(broker(), shared_info, *feedback_cell.value())) { out.bytecode[0] = shared_info.GetBytecodeArray(); } out.num_functions = 1; diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc index a90a2e571f..382f510dd7 100644 --- a/src/compiler/js-inlining.cc +++ b/src/compiler/js-inlining.cc @@ -431,15 +431,19 @@ Reduction JSInliner::ReduceJSCall(Node* node) { shared_info->object()); } - TRACE("Inlining " << *shared_info << " into " << outer_shared_info - << ((exception_target != nullptr) ? " (inside try-block)" - : "")); // Determine the target's feedback vector and its context. Node* context; FeedbackCellRef feedback_cell = DetermineCallContext(node, &context); - CHECK(broker()->IsSerializedForCompilation( - *shared_info, feedback_cell.value().AsFeedbackVector())); + if (!broker()->IsSerializedForCompilation(*shared_info, + *feedback_cell.value())) { + TRACE("Not inlining " << *shared_info << " into " << outer_shared_info + << " because it wasn't serialized for compilation."); + return NoChange(); + } + TRACE("Inlining " << *shared_info << " into " << outer_shared_info + << ((exception_target != nullptr) ? " (inside try-block)" + : "")); // ---------------------------------------------------------------- // After this point, we've made a decision to inline this function. // We shall not bailout from inlining if we got here. diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 5f2fe6e7fc..c8b3b73447 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -1713,7 +1713,10 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) { shared = SharedFunctionInfoRef(broker(), ccp.shared_info()); } else if (target->opcode() == IrOpcode::kCheckClosure) { FeedbackCellRef cell(broker(), FeedbackCellOf(target->op())); - shared = cell.value().AsFeedbackVector().shared_function_info(); + base::Optional feedback_vector = cell.value(); + if (feedback_vector.has_value()) { + shared = feedback_vector->shared_function_info(); + } } if (shared.has_value()) { diff --git a/src/compiler/serializer-for-background-compilation.cc b/src/compiler/serializer-for-background-compilation.cc index 0e97df6d09..57bf4809a4 100644 --- a/src/compiler/serializer-for-background-compilation.cc +++ b/src/compiler/serializer-for-background-compilation.cc @@ -2142,10 +2142,8 @@ void SerializerForBackgroundCompilation::ProcessCallOrConstruct( callee.AddConstant(target->object(), zone(), broker()); } else { // Call; target is feedback cell or callee. - if (target->IsFeedbackCell() && - target->AsFeedbackCell().value().IsFeedbackVector()) { - FeedbackVectorRef vector = - target->AsFeedbackCell().value().AsFeedbackVector(); + if (target->IsFeedbackCell() && target->AsFeedbackCell().value()) { + FeedbackVectorRef vector = *target->AsFeedbackCell().value(); vector.Serialize(); VirtualClosure virtual_closure( vector.shared_function_info().object(), vector.object(), diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc index f832da2e95..aa4d155539 100644 --- a/src/diagnostics/objects-printer.cc +++ b/src/diagnostics/objects-printer.cc @@ -1054,6 +1054,8 @@ void FeedbackVector::FeedbackVectorPrint(std::ostream& os) { // NOLINT os << "\n - optimization tier: " << optimization_tier(); os << "\n - invocation count: " << invocation_count(); os << "\n - profiler ticks: " << profiler_ticks(); + os << "\n - closure feedback cell array: "; + closure_feedback_cell_array().ClosureFeedbackCellArrayPrint(os); FeedbackMetadataIterator iter(metadata()); while (iter.HasNext()) { @@ -1421,6 +1423,9 @@ void JSFunction::JSFunctionPrint(std::ostream& os) { // NOLINT os << "feedback metadata is not available in SFI\n"; } else if (has_feedback_vector()) { feedback_vector().FeedbackVectorPrint(os); + } else if (has_closure_feedback_cell_array()) { + os << "No feedback vector, but we have a closure feedback cell array\n"; + closure_feedback_cell_array().ClosureFeedbackCellArrayPrint(os); } else { os << "not available\n"; } diff --git a/src/objects/feedback-cell-inl.h b/src/objects/feedback-cell-inl.h index 494a951ce4..1a892228f9 100644 --- a/src/objects/feedback-cell-inl.h +++ b/src/objects/feedback-cell-inl.h @@ -21,6 +21,8 @@ namespace internal { TQ_OBJECT_CONSTRUCTORS_IMPL(FeedbackCell) +RELEASE_ACQUIRE_ACCESSORS(FeedbackCell, value, HeapObject, kValueOffset) + void FeedbackCell::clear_padding() { if (FeedbackCell::kAlignedSize == FeedbackCell::kUnalignedSize) return; DCHECK_GE(FeedbackCell::kAlignedSize, FeedbackCell::kUnalignedSize); diff --git a/src/objects/feedback-cell.h b/src/objects/feedback-cell.h index 19f1075e62..ad9653059c 100644 --- a/src/objects/feedback-cell.h +++ b/src/objects/feedback-cell.h @@ -28,6 +28,11 @@ class FeedbackCell : public TorqueGeneratedFeedbackCell { static const int kUnalignedSize = kSize; static const int kAlignedSize = RoundUp(int{kSize}); + using TorqueGeneratedFeedbackCell::value; + using TorqueGeneratedFeedbackCell::set_value; + + DECL_RELEASE_ACQUIRE_ACCESSORS(value, HeapObject) + inline void clear_padding(); inline void reset_feedback_vector( base::Optional function) { if (function->raw_feedback_cell() == isolate->heap()->many_closures_cell()) { Handle feedback_cell = isolate->factory()->NewOneClosureCell(feedback_cell_array); - function->set_raw_feedback_cell(*feedback_cell); + function->set_raw_feedback_cell(*feedback_cell, kReleaseStore); } else { - function->raw_feedback_cell().set_value(*feedback_cell_array); + function->raw_feedback_cell().set_value(*feedback_cell_array, + kReleaseStore); } } @@ -310,7 +311,7 @@ void JSFunction::EnsureFeedbackVector(Handle function, // for more details. DCHECK(function->raw_feedback_cell() != isolate->heap()->many_closures_cell()); - function->raw_feedback_cell().set_value(*feedback_vector); + function->raw_feedback_cell().set_value(*feedback_vector, kReleaseStore); function->raw_feedback_cell().SetInterruptBudget(); } diff --git a/src/objects/js-function.h b/src/objects/js-function.h index 52b0c8a9e3..2340a2e663 100644 --- a/src/objects/js-function.h +++ b/src/objects/js-function.h @@ -165,6 +165,11 @@ class JSFunction : public JSFunctionOrBoundFunction { // the JSFunction's bytecode being flushed. DECL_ACCESSORS(raw_feedback_cell, FeedbackCell) + // [raw_feedback_cell] (synchronized version) When this is initialized from a + // newly allocated object (instead of a root sentinel), it should + // be written with release store semantics. + DECL_RELEASE_ACQUIRE_ACCESSORS(raw_feedback_cell, FeedbackCell) + // Functions related to feedback vector. feedback_vector() can be used once // the function has feedback vectors allocated. feedback vectors may not be // available after compile when lazily allocating feedback vectors.