[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

@@ -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> object) {
   return constant_array_builder()->Insert(object);
 }
 
+size_t BytecodeArrayBuilder::AllocateConstantPoolEntry() {
+  return constant_array_builder()->AllocateEntry();
+}
+
+void BytecodeArrayBuilder::InsertConstantPoolEntryAt(size_t entry,
+                                                     Handle<Object> object) {
+  constant_array_builder()->InsertAllocatedEntry(entry, object);
+}
+
 void BytecodeArrayBuilder::SetReturnPosition() {
   if (return_position_ == kNoSourcePosition) return;
   latest_source_info_.MakeStatementPosition(return_position_);

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> 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> object);
+
   void InitializeReturnPosition(FunctionLiteral* literal);
 
   void SetStatementPosition(Statement* stmt);

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<Object> initial_value) {
+    DCHECK(!slot.IsInvalid());
+    declaration_pairs_.push_back(handle(Smi::FromInt(slot.ToInt()), isolate_));
+    declaration_pairs_.push_back(initial_value);
+  }
+
+  Handle<FixedArray> AllocateDeclarationPairs() {
+    DCHECK(has_constant_pool_entry_);
+    int array_index = 0;
+    Handle<FixedArray> data = isolate_->factory()->NewFixedArray(
+        static_cast<int>(declaration_pairs_.size()), TENURED);
+    for (Handle<Object> 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<Handle<Object>> 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<BytecodeArray> BytecodeGenerator::MakeBytecode() {
+  GenerateBytecode();
+
+  // Build global declaration pair arrays.
+  for (GlobalDeclarationsBuilder* globals_builder : global_declarations_) {
+    Handle<FixedArray> 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<BytecodeArray> 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<BytecodeArray> 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<Declaration*>* 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<FixedArray> data = isolate()->factory()->NewFixedArray(
-      static_cast<int>(globals()->size()), TENURED);
-  for (Handle<Object> 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<Statement*>* statements) {

src/interpreter/bytecode-generator.h

@@ -35,6 +35,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   void VisitStatements(ZoneList<Statement*>* statments);
 
  private:
+  class AccumulatorResultScope;
   class ContextScope;
   class ControlScope;
   class ControlScopeForBreakable;
@@ -44,11 +45,12 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   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<BytecodeGenerator> {
     return register_allocator_;
   }
 
-  ZoneVector<Handle<Object>>* 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<BytecodeGenerator> {
   BytecodeArrayBuilder* builder_;
   CompilationInfo* info_;
   Scope* scope_;
-  ZoneVector<Handle<Object>> globals_;
+  GlobalDeclarationsBuilder* globals_builder_;
+  ZoneVector<GlobalDeclarationsBuilder*> global_declarations_;
   ControlScope* execution_control_;
   ContextScope* execution_context_;
   ExpressionResultScope* execution_result_;

src/interpreter/constant-array-builder.cc

@@ -46,6 +46,13 @@ Handle<Object> ConstantArrayBuilder::ConstantArraySlice::At(
   return constants_[index - start_index()];
 }
 
+void ConstantArrayBuilder::ConstantArraySlice::InsertAt(size_t index,
+                                                        Handle<Object> 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,28 +136,21 @@ size_t ConstantArrayBuilder::Insert(Handle<Object> object) {
 ConstantArrayBuilder::index_t ConstantArrayBuilder::AllocateEntry(
     Handle<Object> object) {
   DCHECK(!object->IsOddball());
+  index_t index = AllocateIndex(object);
   index_t* entry = constants_map()->Get(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_t>(index);
-      return *entry;
-      break;
-    }
-  }
-  UNREACHABLE();
-  return kMaxUInt32;
+  *entry = index;
+  return index;
 }
 
-OperandSize ConstantArrayBuilder::CreateReservedEntry() {
+ConstantArrayBuilder::index_t ConstantArrayBuilder::AllocateIndex(
+    Handle<Object> object) {
   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();
+      return static_cast<index_t>(idx_slice_[i]->Allocate(object));
     }
   }
   UNREACHABLE();
-  return OperandSize::kNone;
+  return kMaxUInt32;
 }
 
 ConstantArrayBuilder::ConstantArraySlice*
@@ -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> 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> object) {
   DiscardReservedEntry(operand_size);

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> 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> 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> object);
+  index_t AllocateIndex(Handle<Object> 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> object);
     Handle<Object> At(size_t index) const;
+    void InsertAt(size_t index, Handle<Object> 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<index_t>* constants_map() { return &constants_map_; }

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<int>(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<Object> empty = builder.At(i);
+      CHECK(empty->SameValue(isolate()->heap()->the_hole_value()));
+    } else {
+      CHECK_EQ(Handle<Smi>::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<int>(i)), isolate()));
+  }
+
+  // Check values after reserved entries are inserted.
+  for (size_t i = 0; i < k16BitCapacity; i++) {
+    CHECK_EQ(Handle<Smi>::cast(builder.At(i))->value(), i);
+  }
+}
+
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8