From 8d4658077ca9f35904cefe0fb1cb4e927ea9f240 Mon Sep 17 00:00:00 2001 From: rmcilroy Date: Mon, 25 Jul 2016 04:30:36 -0700 Subject: [PATCH] [Interpreter] Avoid allocating pairs array in VisitDeclarations. Move the logic for allocating the global declaration pair array from VisitDeclarations to a later step. This is required for concurrent bytecode generation. This change requires adding support for reserving fixed constant pool array entries, which can be later updated with the value of the literal. BUG=v8:5203 Review-Url: https://codereview.chromium.org/2167763003 Cr-Commit-Position: refs/heads/master@{#38010} --- src/interpreter/bytecode-array-builder.cc | 15 +++ src/interpreter/bytecode-array-builder.h | 6 + src/interpreter/bytecode-generator.cc | 114 +++++++++++++----- src/interpreter/bytecode-generator.h | 11 +- src/interpreter/constant-array-builder.cc | 58 ++++++--- src/interpreter/constant-array-builder.h | 11 +- .../constant-array-builder-unittest.cc | 34 ++++++ 7 files changed, 199 insertions(+), 50 deletions(-) diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc index 7e7a142538..1aefe7af49 100644 --- a/src/interpreter/bytecode-array-builder.cc +++ b/src/interpreter/bytecode-array-builder.cc @@ -179,6 +179,12 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CompareOperation(Token::Value op, return *this; } +BytecodeArrayBuilder& BytecodeArrayBuilder::LoadConstantPoolEntry( + size_t entry) { + Output(Bytecode::kLdaConstant, UnsignedOperand(entry)); + return *this; +} + BytecodeArrayBuilder& BytecodeArrayBuilder::LoadLiteral( v8::internal::Smi* smi) { int32_t raw_smi = smi->value(); @@ -633,6 +639,15 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle object) { return constant_array_builder()->Insert(object); } +size_t BytecodeArrayBuilder::AllocateConstantPoolEntry() { + return constant_array_builder()->AllocateEntry(); +} + +void BytecodeArrayBuilder::InsertConstantPoolEntryAt(size_t entry, + Handle object) { + constant_array_builder()->InsertAllocatedEntry(entry, object); +} + void BytecodeArrayBuilder::SetReturnPosition() { if (return_position_ == kNoSourcePosition) return; latest_source_info_.MakeStatementPosition(return_position_); diff --git a/src/interpreter/bytecode-array-builder.h b/src/interpreter/bytecode-array-builder.h index 92d1b92e85..8d6a2fb5a7 100644 --- a/src/interpreter/bytecode-array-builder.h +++ b/src/interpreter/bytecode-array-builder.h @@ -79,6 +79,7 @@ class BytecodeArrayBuilder final : public ZoneObject { bool TemporaryRegisterIsLive(Register reg) const; // Constant loads to accumulator. + BytecodeArrayBuilder& LoadConstantPoolEntry(size_t entry); BytecodeArrayBuilder& LoadLiteral(v8::internal::Smi* value); BytecodeArrayBuilder& LoadLiteral(Handle object); BytecodeArrayBuilder& LoadUndefined(); @@ -260,6 +261,11 @@ class BytecodeArrayBuilder final : public ZoneObject { // entry, so that it can be referenced by above exception handling support. int NewHandlerEntry() { return handler_table_builder()->NewHandlerEntry(); } + // Allocates a slot in the constant pool which can later be inserted. + size_t AllocateConstantPoolEntry(); + // Inserts a entry into an allocated constant pool entry. + void InsertConstantPoolEntryAt(size_t entry, Handle object); + void InitializeReturnPosition(FunctionLiteral* literal); void SetStatementPosition(Statement* stmt); diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 6c51499740..3b5b12edfc 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -532,6 +532,53 @@ class BytecodeGenerator::RegisterResultScope final Register result_register_; }; +// Used to build a list of global declaration initial value pairs. +class BytecodeGenerator::GlobalDeclarationsBuilder final : public ZoneObject { + public: + GlobalDeclarationsBuilder(Isolate* isolate, Zone* zone) + : isolate_(isolate), + declaration_pairs_(0, zone), + constant_pool_entry_(0), + has_constant_pool_entry_(false) {} + + void AddDeclaration(FeedbackVectorSlot slot, Handle initial_value) { + DCHECK(!slot.IsInvalid()); + declaration_pairs_.push_back(handle(Smi::FromInt(slot.ToInt()), isolate_)); + declaration_pairs_.push_back(initial_value); + } + + Handle AllocateDeclarationPairs() { + DCHECK(has_constant_pool_entry_); + int array_index = 0; + Handle data = isolate_->factory()->NewFixedArray( + static_cast(declaration_pairs_.size()), TENURED); + for (Handle obj : declaration_pairs_) { + data->set(array_index++, *obj); + } + return data; + } + + size_t constant_pool_entry() { + DCHECK(has_constant_pool_entry_); + return constant_pool_entry_; + } + + void set_constant_pool_entry(size_t constant_pool_entry) { + DCHECK(!empty()); + DCHECK(!has_constant_pool_entry_); + constant_pool_entry_ = constant_pool_entry; + has_constant_pool_entry_ = true; + } + + bool empty() { return declaration_pairs_.empty(); } + + private: + Isolate* isolate_; + ZoneVector> declaration_pairs_; + size_t constant_pool_entry_; + bool has_constant_pool_entry_; +}; + BytecodeGenerator::BytecodeGenerator(CompilationInfo* info) : isolate_(info->isolate()), zone_(info->zone()), @@ -542,7 +589,9 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info) info->SourcePositionRecordingMode())), info_(info), scope_(info->scope()), - globals_(0, info->zone()), + globals_builder_(new (zone()) GlobalDeclarationsBuilder(info->isolate(), + info->zone())), + global_declarations_(0, info->zone()), execution_control_(nullptr), execution_context_(nullptr), execution_result_(nullptr), @@ -553,6 +602,20 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info) } Handle BytecodeGenerator::MakeBytecode() { + GenerateBytecode(); + + // Build global declaration pair arrays. + for (GlobalDeclarationsBuilder* globals_builder : global_declarations_) { + Handle declarations = + globals_builder->AllocateDeclarationPairs(); + builder()->InsertConstantPoolEntryAt(globals_builder->constant_pool_entry(), + declarations); + } + + return builder()->ToBytecodeArray(); +} + +void BytecodeGenerator::GenerateBytecode() { // Initialize the incoming context. ContextScope incoming_context(this, scope(), false); @@ -572,9 +635,9 @@ Handle BytecodeGenerator::MakeBytecode() { VisitNewLocalFunctionContext(); ContextScope local_function_context(this, scope(), false); VisitBuildLocalActivationContext(); - MakeBytecodeBody(); + GenerateBytecodeBody(); } else { - MakeBytecodeBody(); + GenerateBytecodeBody(); } // In generator functions, we may not have visited every yield in the AST @@ -586,10 +649,9 @@ Handle BytecodeGenerator::MakeBytecode() { } builder()->EnsureReturn(); - return builder()->ToBytecodeArray(); } -void BytecodeGenerator::MakeBytecodeBody() { +void BytecodeGenerator::GenerateBytecodeBody() { // Build the arguments object if it is used. VisitArgumentsObject(scope()->arguments()); @@ -723,9 +785,8 @@ void BytecodeGenerator::VisitVariableDeclaration(VariableDeclaration* decl) { case VariableLocation::UNALLOCATED: { DCHECK(!variable->binding_needs_init()); FeedbackVectorSlot slot = decl->proxy()->VariableFeedbackSlot(); - DCHECK(!slot.IsInvalid()); - globals()->push_back(handle(Smi::FromInt(slot.ToInt()), isolate())); - globals()->push_back(isolate()->factory()->undefined_value()); + globals_builder()->AddDeclaration( + slot, isolate()->factory()->undefined_value()); break; } case VariableLocation::LOCAL: @@ -773,9 +834,7 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) { // Check for stack-overflow exception. if (function.is_null()) return SetStackOverflow(); FeedbackVectorSlot slot = decl->proxy()->VariableFeedbackSlot(); - DCHECK(!slot.IsInvalid()); - globals()->push_back(handle(Smi::FromInt(slot.ToInt()), isolate())); - globals()->push_back(function); + globals_builder()->AddDeclaration(slot, function); break; } case VariableLocation::PARAMETER: @@ -810,34 +869,35 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) { void BytecodeGenerator::VisitDeclarations( ZoneList* declarations) { RegisterAllocationScope register_scope(this); - DCHECK(globals()->empty()); + DCHECK(globals_builder()->empty()); for (int i = 0; i < declarations->length(); i++) { RegisterAllocationScope register_scope(this); Visit(declarations->at(i)); } - if (globals()->empty()) return; - int array_index = 0; - Handle data = isolate()->factory()->NewFixedArray( - static_cast(globals()->size()), TENURED); - for (Handle obj : *globals()) data->set(array_index++, *obj); + if (globals_builder()->empty()) return; + + globals_builder()->set_constant_pool_entry( + builder()->AllocateConstantPoolEntry()); int encoded_flags = info()->GetDeclareGlobalsFlags(); register_allocator()->PrepareForConsecutiveAllocations(3); Register pairs = register_allocator()->NextConsecutiveRegister(); - builder()->LoadLiteral(data); - builder()->StoreAccumulatorInRegister(pairs); - Register flags = register_allocator()->NextConsecutiveRegister(); - builder()->LoadLiteral(Smi::FromInt(encoded_flags)); - builder()->StoreAccumulatorInRegister(flags); - DCHECK(flags.index() == pairs.index() + 1); - Register function = register_allocator()->NextConsecutiveRegister(); - builder()->MoveRegister(Register::function_closure(), function); - builder()->CallRuntime(Runtime::kDeclareGlobalsForInterpreter, pairs, 3); - globals()->clear(); + // Emit code to declare globals. + builder() + ->LoadConstantPoolEntry(globals_builder()->constant_pool_entry()) + .StoreAccumulatorInRegister(pairs) + .LoadLiteral(Smi::FromInt(encoded_flags)) + .StoreAccumulatorInRegister(flags) + .MoveRegister(Register::function_closure(), function) + .CallRuntime(Runtime::kDeclareGlobalsForInterpreter, pairs, 3); + + // Push and reset globals builder. + global_declarations_.push_back(globals_builder()); + globals_builder_ = new (zone()) GlobalDeclarationsBuilder(isolate(), zone()); } void BytecodeGenerator::VisitStatements(ZoneList* statements) { diff --git a/src/interpreter/bytecode-generator.h b/src/interpreter/bytecode-generator.h index 744971fbf9..67e3a05ba2 100644 --- a/src/interpreter/bytecode-generator.h +++ b/src/interpreter/bytecode-generator.h @@ -35,6 +35,7 @@ class BytecodeGenerator final : public AstVisitor { void VisitStatements(ZoneList* statments); private: + class AccumulatorResultScope; class ContextScope; class ControlScope; class ControlScopeForBreakable; @@ -44,11 +45,12 @@ class BytecodeGenerator final : public AstVisitor { class ControlScopeForTryFinally; class ExpressionResultScope; class EffectResultScope; - class AccumulatorResultScope; + class GlobalDeclarationsBuilder; class RegisterResultScope; class RegisterAllocationScope; - void MakeBytecodeBody(); + void GenerateBytecode(); + void GenerateBytecodeBody(); DEFINE_AST_VISITOR_SUBCLASS_MEMBERS(); @@ -196,7 +198,7 @@ class BytecodeGenerator final : public AstVisitor { return register_allocator_; } - ZoneVector>* globals() { return &globals_; } + GlobalDeclarationsBuilder* globals_builder() { return globals_builder_; } inline LanguageMode language_mode() const; int feedback_index(FeedbackVectorSlot slot) const; @@ -205,7 +207,8 @@ class BytecodeGenerator final : public AstVisitor { BytecodeArrayBuilder* builder_; CompilationInfo* info_; Scope* scope_; - ZoneVector> globals_; + GlobalDeclarationsBuilder* globals_builder_; + ZoneVector global_declarations_; ControlScope* execution_control_; ContextScope* execution_context_; ExpressionResultScope* execution_result_; diff --git a/src/interpreter/constant-array-builder.cc b/src/interpreter/constant-array-builder.cc index 7ce50b580e..1643e4c6a8 100644 --- a/src/interpreter/constant-array-builder.cc +++ b/src/interpreter/constant-array-builder.cc @@ -46,6 +46,13 @@ Handle ConstantArrayBuilder::ConstantArraySlice::At( return constants_[index - start_index()]; } +void ConstantArrayBuilder::ConstantArraySlice::InsertAt(size_t index, + Handle object) { + DCHECK_GE(index, start_index()); + DCHECK_LT(index, start_index() + size()); + constants_[index - start_index()] = object; +} + STATIC_CONST_MEMBER_DEFINITION const size_t ConstantArrayBuilder::k8BitCapacity; STATIC_CONST_MEMBER_DEFINITION const size_t ConstantArrayBuilder::k16BitCapacity; @@ -73,9 +80,9 @@ size_t ConstantArrayBuilder::size() const { return idx_slice_[0]->size(); } -const ConstantArrayBuilder::ConstantArraySlice* -ConstantArrayBuilder::IndexToSlice(size_t index) const { - for (const ConstantArraySlice* slice : idx_slice_) { +ConstantArrayBuilder::ConstantArraySlice* ConstantArrayBuilder::IndexToSlice( + size_t index) const { + for (ConstantArraySlice* slice : idx_slice_) { if (index <= slice->max_index()) { return slice; } @@ -129,30 +136,23 @@ size_t ConstantArrayBuilder::Insert(Handle object) { ConstantArrayBuilder::index_t ConstantArrayBuilder::AllocateEntry( Handle object) { DCHECK(!object->IsOddball()); + index_t index = AllocateIndex(object); index_t* entry = constants_map()->Get(object); + *entry = index; + return index; +} + +ConstantArrayBuilder::index_t ConstantArrayBuilder::AllocateIndex( + Handle object) { for (size_t i = 0; i < arraysize(idx_slice_); ++i) { if (idx_slice_[i]->available() > 0) { - size_t index = idx_slice_[i]->Allocate(object); - *entry = static_cast(index); - return *entry; - break; + return static_cast(idx_slice_[i]->Allocate(object)); } } UNREACHABLE(); return kMaxUInt32; } -OperandSize ConstantArrayBuilder::CreateReservedEntry() { - for (size_t i = 0; i < arraysize(idx_slice_); ++i) { - if (idx_slice_[i]->available() > 0) { - idx_slice_[i]->Reserve(); - return idx_slice_[i]->operand_size(); - } - } - UNREACHABLE(); - return OperandSize::kNone; -} - ConstantArrayBuilder::ConstantArraySlice* ConstantArrayBuilder::OperandSizeToSlice(OperandSize operand_size) const { ConstantArraySlice* slice = nullptr; @@ -174,6 +174,28 @@ ConstantArrayBuilder::OperandSizeToSlice(OperandSize operand_size) const { return slice; } +size_t ConstantArrayBuilder::AllocateEntry() { + return AllocateIndex(isolate_->factory()->the_hole_value()); +} + +void ConstantArrayBuilder::InsertAllocatedEntry(size_t index, + Handle object) { + DCHECK_EQ(isolate_->heap()->the_hole_value(), *At(index)); + ConstantArraySlice* slice = IndexToSlice(index); + slice->InsertAt(index, object); +} + +OperandSize ConstantArrayBuilder::CreateReservedEntry() { + for (size_t i = 0; i < arraysize(idx_slice_); ++i) { + if (idx_slice_[i]->available() > 0) { + idx_slice_[i]->Reserve(); + return idx_slice_[i]->operand_size(); + } + } + UNREACHABLE(); + return OperandSize::kNone; +} + size_t ConstantArrayBuilder::CommitReservedEntry(OperandSize operand_size, Handle object) { DiscardReservedEntry(operand_size); diff --git a/src/interpreter/constant-array-builder.h b/src/interpreter/constant-array-builder.h index 1a68646251..2e4445af41 100644 --- a/src/interpreter/constant-array-builder.h +++ b/src/interpreter/constant-array-builder.h @@ -48,6 +48,13 @@ class ConstantArrayBuilder final BASE_EMBEDDED { // present. Returns the array index associated with the object. size_t Insert(Handle object); + // Allocates an empty entry and returns the array index associated with the + // reservation. Entry can be inserted by calling InsertReservedEntry(). + size_t AllocateEntry(); + + // Inserts the given object into an allocated entry. + void InsertAllocatedEntry(size_t index, Handle object); + // Creates a reserved entry in the constant pool and returns // the size of the operand that'll be required to hold the entry // when committed. @@ -64,6 +71,7 @@ class ConstantArrayBuilder final BASE_EMBEDDED { typedef uint32_t index_t; index_t AllocateEntry(Handle object); + index_t AllocateIndex(Handle object); struct ConstantArraySlice final : public ZoneObject { ConstantArraySlice(Zone* zone, size_t start_index, size_t capacity, @@ -72,6 +80,7 @@ class ConstantArrayBuilder final BASE_EMBEDDED { void Unreserve(); size_t Allocate(Handle object); Handle At(size_t index) const; + void InsertAt(size_t index, Handle object); inline size_t available() const { return capacity() - reserved() - size(); } inline size_t reserved() const { return reserved_; } @@ -91,7 +100,7 @@ class ConstantArrayBuilder final BASE_EMBEDDED { DISALLOW_COPY_AND_ASSIGN(ConstantArraySlice); }; - const ConstantArraySlice* IndexToSlice(size_t index) const; + ConstantArraySlice* IndexToSlice(size_t index) const; ConstantArraySlice* OperandSizeToSlice(OperandSize operand_size) const; IdentityMap* constants_map() { return &constants_map_; } diff --git a/test/unittests/interpreter/constant-array-builder-unittest.cc b/test/unittests/interpreter/constant-array-builder-unittest.cc index c48ac58738..8bee2452da 100644 --- a/test/unittests/interpreter/constant-array-builder-unittest.cc +++ b/test/unittests/interpreter/constant-array-builder-unittest.cc @@ -283,6 +283,40 @@ TEST_F(ConstantArrayBuilderTest, ReservationsAtAllScales) { } } +TEST_F(ConstantArrayBuilderTest, AllocateEntriesWithFixedReservations) { + ConstantArrayBuilder builder(isolate(), zone()); + for (size_t i = 0; i < k16BitCapacity; i++) { + if ((i % 2) == 0) { + CHECK_EQ(i, builder.AllocateEntry()); + } else { + builder.Insert(handle(Smi::FromInt(static_cast(i)), isolate())); + } + } + CHECK_EQ(builder.size(), k16BitCapacity); + + // Check values before reserved entries are inserted. + for (size_t i = 0; i < k16BitCapacity; i++) { + if ((i % 2) == 0) { + // Check reserved values are the hole. + Handle empty = builder.At(i); + CHECK(empty->SameValue(isolate()->heap()->the_hole_value())); + } else { + CHECK_EQ(Handle::cast(builder.At(i))->value(), i); + } + } + + // Insert reserved entries. + for (size_t i = 0; i < k16BitCapacity; i += 2) { + builder.InsertAllocatedEntry( + i, handle(Smi::FromInt(static_cast(i)), isolate())); + } + + // Check values after reserved entries are inserted. + for (size_t i = 0; i < k16BitCapacity; i++) { + CHECK_EQ(Handle::cast(builder.At(i))->value(), i); + } +} + } // namespace interpreter } // namespace internal } // namespace v8