Reviewers: Hannes Payer,

Message:
Hannes, could you have a quick look please (this is needed to green the tree).
Thanks!

Description:
[interpreter] Fix nosnap build for interpreter table generation.

Moves the creation of the interpreter table early on during initialization
to ensure that even on nosnap builds it still gets allocated in the
first page.

BUG=v8:4280
LOG=N

Please review this at https://codereview.chromium.org/1278413002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+47, -27 lines):
  M src/heap/heap.h
  M src/heap/heap.cc
  M src/interpreter/interpreter.h
  M src/interpreter/interpreter.cc
  M test/cctest/interpreter/test-interpreter.cc


Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index eb18925b20e6b430fb18d89038195fe496fe9673..225fd89bd5b8dd4e9df4346ee54b23a8df0869cf 100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -25,6 +25,7 @@
 #include "src/heap/objects-visiting.h"
 #include "src/heap/store-buffer.h"
 #include "src/heap-profiler.h"
+#include "src/interpreter/interpreter.h"
 #include "src/runtime-profiler.h"
 #include "src/scopeinfo.h"
 #include "src/snapshot/natives.h"
@@ -3362,7 +3363,9 @@ void Heap::CreateInitialObjects() {
   set_weak_stack_trace_list(Smi::FromInt(0));

   // Will be filled in by Interpreter::Initialize().
-  set_interpreter_table(empty_fixed_array());
+  set_interpreter_table(
+      *interpreter::Interpreter::CreateUninitializedInterpreterTable(
+          isolate()));

   set_allocation_sites_scratchpad(
       *factory->NewFixedArray(kAllocationSiteScratchpadSize, TENURED));
@@ -3414,7 +3417,6 @@ bool Heap::RootCanBeWrittenAfterInitialization(Heap::RootListIndex root_index) {
     case kWeakObjectToCodeTableRootIndex:
     case kRetainedMapsRootIndex:
     case kWeakStackTraceListRootIndex:
-    case kInterpreterTableRootIndex:
 // Smi values
 #define SMI_ENTRY(type, name, Name) case k##Name##RootIndex:
       SMI_ROOT_LIST(SMI_ENTRY)
Index: src/heap/heap.h
diff --git a/src/heap/heap.h b/src/heap/heap.h
index 561d24cc2ff9e0a6f934c0c969882c4c1d86f3c5..4a1c1f8b1faa90eb5b08f9e3fd9eaab90378dbf6 100644
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -1024,10 +1024,6 @@ class Heap {
     roots_[kMaterializedObjectsRootIndex] = objects;
   }

-  void public_set_interpreter_table(FixedArray* table) {
-    roots_[kInterpreterTableRootIndex] = table;
-  }
-
   // Generated code can embed this address to get access to the roots.
   Object** roots_array_start() { return roots_; }

Index: src/interpreter/interpreter.cc
diff --git a/src/interpreter/interpreter.cc b/src/interpreter/interpreter.cc
index 3d37874075575fcda79404b63d0a4f1c01713791..6ffbe48e1930024d58795236e9ac7fa3427eff64 100644
--- a/src/interpreter/interpreter.cc
+++ b/src/interpreter/interpreter.cc
@@ -18,33 +18,48 @@ using compiler::Node;
 #define __ assembler->


-Interpreter::Interpreter(Isolate* isolate) : isolate_(isolate) {}
+Interpreter::Interpreter(Isolate* isolate)
+    : isolate_(isolate), initialized_(false) {}
+
+
+// static
+Handle<FixedArray> Interpreter::CreateUninitializedInterpreterTable(
+    Isolate* isolate) {
+  Handle<FixedArray> handler_table = isolate->factory()->NewFixedArray(
+      static_cast<int>(Bytecode::kLast) + 1, TENURED);
+ // We rely on the interpreter handler table being immovable, so check that
+  // it was allocated on the first page (which is always immovable).
+  DCHECK(isolate->heap()->old_space()->FirstPage()->Contains(
+      handler_table->address()));
+  for (int i = 0; i < static_cast<int>(Bytecode::kLast); i++) {
+ handler_table->set(i, isolate->builtins()->builtin(Builtins::kIllegal));
+  }
+  return handler_table;
+}


 void Interpreter::Initialize(bool create_heap_objects) {
   DCHECK(FLAG_ignition);
+  if (initialized_) return;
+
   if (create_heap_objects) {
     Zone zone;
     HandleScope scope(isolate_);
-    Handle<FixedArray> handler_table = isolate_->factory()->NewFixedArray(
-        static_cast<int>(Bytecode::kLast) + 1, TENURED);
- // We rely on the interpreter handler table being immovable, so check that
-    // it was allocated on the first page (which is always immovable).
-    DCHECK(isolate_->heap()->old_space()->FirstPage()->Contains(
-        handler_table->address()));
-    isolate_->heap()->public_set_interpreter_table(*handler_table);
-
-#define GENERATE_CODE(Name, ...)                                    \
-  {                                                                 \
-    compiler::InterpreterAssembler assembler(isolate_, &zone,       \
-                                             Bytecode::k##Name);    \
-    Do##Name(&assembler);                                           \
-    Handle<Code> code = assembler.GenerateCode();                   \
-    handler_table->set(static_cast<int>(Bytecode::k##Name), *code); \
-  }
+ Handle<FixedArray> handler_table = isolate_->factory()->interpreter_table();
+
+#define GENERATE_CODE(Name, ...)                                      \
+    {                                                                 \
+      compiler::InterpreterAssembler assembler(isolate_, &zone,       \
+                                               Bytecode::k##Name);    \
+      Do##Name(&assembler);                                           \
+      Handle<Code> code = assembler.GenerateCode();                   \
+      handler_table->set(static_cast<int>(Bytecode::k##Name), *code); \
+    }
     BYTECODE_LIST(GENERATE_CODE)
 #undef GENERATE_CODE
   }
+
+  initialized_ = true;
 }


Index: src/interpreter/interpreter.h
diff --git a/src/interpreter/interpreter.h b/src/interpreter/interpreter.h
index 1e5b00288261822e71f31c00e4c5f5309a13369d..1a241ad21d24e9939ece183d70dd45a11754fa4e 100644
--- a/src/interpreter/interpreter.h
+++ b/src/interpreter/interpreter.h
@@ -28,6 +28,15 @@ class Interpreter {
   explicit Interpreter(Isolate* isolate);
   virtual ~Interpreter() {}

+  bool initialized() { return initialized_; }
+
+  // Creates an uninitialized interpreter handler table, where each handler
+  // points to the Illegal builtin.
+  static Handle<FixedArray> CreateUninitializedInterpreterTable(
+      Isolate* isolate);
+
+  // Initializes the interpreter, generating heap objects if
+  // |create_heap_objects| is true.
   void Initialize(bool create_heap_objects);

  private:
@@ -38,6 +47,7 @@ class Interpreter {
 #undef DECLARE_BYTECODE_HANDLER_GENERATOR

   Isolate* isolate_;
+  bool initialized_;

   DISALLOW_COPY_AND_ASSIGN(Interpreter);
 };
Index: test/cctest/interpreter/test-interpreter.cc
diff --git a/test/cctest/interpreter/test-interpreter.cc b/test/cctest/interpreter/test-interpreter.cc index 183348d3ce3943f44c709723ff7ec92a324a9a89..a5b6ed8b36d87c5214cdf9cbd6e2625d8887feca 100644
--- a/test/cctest/interpreter/test-interpreter.cc
+++ b/test/cctest/interpreter/test-interpreter.cc
@@ -35,10 +35,7 @@ class InterpreterTester {
   InterpreterTester(Isolate* isolate, Handle<BytecodeArray> bytecode)
: isolate_(isolate), function_(GetBytecodeFunction(isolate, bytecode)) {
     i::FLAG_ignition = true;
- Handle<FixedArray> empty_array = isolate->factory()->empty_fixed_array();
-    Handle<FixedArray> interpreter_table =
-        isolate->factory()->interpreter_table();
-    if (interpreter_table.is_identical_to(empty_array)) {
+    if (!isolate->interpreter()->initialized()) {
       // Ensure handler table is generated.
       isolate->interpreter()->Initialize(true);
     }


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to