[wasm] Clarify source of runtime information for interpreter.

This is part of the effort to consolidate the ownership of
wasm instantiation/specialization parameters.

This change is focused solely on the interpreter part of that effort, to
verify we're not regressing performance in interpreter benchmarks.

There are two aspects being addressed:
- dataflow-wise, we always fetch the interpreter's memory view from the
runtime objects (i.e. WasmInstanceObject/WasmCompiledModule). This is
consistent with how other instance-specific information is obtained
(e.g. code, indirect functions).

- representation-wise, we do not reuse ModuleEnv/WasmInstance just for
the memory view, because it is surprising that other instance info isn't
accessed from there. 

Bug: 
Change-Id: I536fbffd8e1f142a315fa1770ba9b08319f56a8e
Reviewed-on: https://chromium-review.googlesource.com/602083Reviewed-by: Ben Titzer <titzer@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Mircea Trofin <mtrofin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47205}

src/wasm/wasm-debug.cc

@@ -95,10 +95,9 @@ InterpreterHandle* GetInterpreterHandle(WasmDebugInfo* debug_info);
 
 class InterpreterHandle {
   MOVE_ONLY_NO_DEFAULT_CONSTRUCTOR(InterpreterHandle);
-
-  WasmInstance instance_;
-  WasmInterpreter interpreter_;
   Isolate* isolate_;
+  const WasmModule* module_;
+  WasmInterpreter interpreter_;
   StepAction next_step_action_ = StepNone;
   int last_step_stack_depth_ = 0;
   std::unordered_map<Address, uint32_t> activations_;
@@ -132,50 +131,53 @@ class InterpreterHandle {
     return {frame_base, frame_limit};
   }
 
- public:
-  // Initialize in the right order, using helper methods to make this possible.
-  // WasmInterpreter has to be allocated in place, since it is not movable.
-  InterpreterHandle(Isolate* isolate, WasmDebugInfo* debug_info,
-                    WasmInstance* external_instance = nullptr)
-      : instance_(debug_info->wasm_instance()->compiled_module()->module()),
-        interpreter_(isolate, GetBytesEnv(external_instance ? external_instance
-                                                            : &instance_,
-                                          debug_info)),
-        isolate_(isolate) {
-    DisallowHeapAllocation no_gc;
-
-    WasmInstanceObject* instance = debug_info->wasm_instance();
-
-    // Set memory start pointer and size.
-    instance_.mem_start = nullptr;
-    instance_.mem_size = 0;
-    if (instance->has_memory_buffer()) {
-      UpdateMemory(instance->memory_buffer());
-    } else {
-      DCHECK_EQ(0, instance_.module->min_mem_pages);
-    }
+  static Vector<const byte> GetBytes(WasmDebugInfo* debug_info) {
+    // Return raw pointer into heap. The WasmInterpreter will make its own copy
+    // of this data anyway, and there is no heap allocation in-between.
+    SeqOneByteString* bytes_str =
+        debug_info->wasm_instance()->compiled_module()->module_bytes();
+    return {bytes_str->GetChars(), static_cast<size_t>(bytes_str->length())};
+  }
 
-    // Set pointer to globals storage.
-    instance_.globals_start =
-        debug_info->wasm_instance()->compiled_module()->GetGlobalsStartOrNull();
+  static uint32_t GetMemSize(WasmDebugInfo* debug_info) {
+    DisallowHeapAllocation no_gc;
+    WasmCompiledModule* compiled_module =
+        debug_info->wasm_instance()->compiled_module();
+    return compiled_module->has_embedded_mem_size()
+               ? compiled_module->embedded_mem_size()
+               : 0;
   }
 
-  ~InterpreterHandle() {
-    DCHECK_EQ(0, activations_.size());
+  static byte* GetMemStart(WasmDebugInfo* debug_info) {
+    DisallowHeapAllocation no_gc;
+    WasmCompiledModule* compiled_module =
+        debug_info->wasm_instance()->compiled_module();
+    return reinterpret_cast<byte*>(compiled_module->has_embedded_mem_start()
+                                       ? compiled_module->embedded_mem_start()
+                                       : 0);
   }
 
-  static ModuleBytesEnv GetBytesEnv(WasmInstance* instance,
-                                    WasmDebugInfo* debug_info) {
-    // Return raw pointer into heap. The WasmInterpreter will make its own copy
-    // of this data anyway, and there is no heap allocation in-between.
-    SeqOneByteString* bytes_str =
-        debug_info->wasm_instance()->compiled_module()->module_bytes();
-    Vector<const byte> bytes(bytes_str->GetChars(), bytes_str->length());
-    return {instance->module, instance, bytes};
+  static byte* GetGlobalsStart(WasmDebugInfo* debug_info) {
+    DisallowHeapAllocation no_gc;
+    WasmCompiledModule* compiled_module =
+        debug_info->wasm_instance()->compiled_module();
+    return reinterpret_cast<byte*>(compiled_module->has_globals_start()
+                                       ? compiled_module->globals_start()
+                                       : 0);
   }
 
+ public:
+  InterpreterHandle(Isolate* isolate, WasmDebugInfo* debug_info)
+      : isolate_(isolate),
+        module_(debug_info->wasm_instance()->compiled_module()->module()),
+        interpreter_(isolate, module_, GetBytes(debug_info),
+                     GetGlobalsStart(debug_info), GetMemStart(debug_info),
+                     GetMemSize(debug_info)) {}
+
+  ~InterpreterHandle() { DCHECK_EQ(0, activations_.size()); }
+
   WasmInterpreter* interpreter() { return &interpreter_; }
-  const WasmModule* module() { return instance_.module; }
+  const WasmModule* module() const { return module_; }
 
   void PrepareStep(StepAction step_action) {
     next_step_action_ = step_action;
@@ -647,11 +649,10 @@ Handle<WasmDebugInfo> WasmDebugInfo::New(Handle<WasmInstanceObject> instance) {
 }
 
 WasmInterpreter* WasmDebugInfo::SetupForTesting(
-    Handle<WasmInstanceObject> instance_obj, WasmInstance* instance) {
+    Handle<WasmInstanceObject> instance_obj) {
   Handle<WasmDebugInfo> debug_info = WasmDebugInfo::New(instance_obj);
   Isolate* isolate = instance_obj->GetIsolate();
-  InterpreterHandle* cpp_handle =
-      new InterpreterHandle(isolate, *debug_info, instance);
+  InterpreterHandle* cpp_handle = new InterpreterHandle(isolate, *debug_info);
   Handle<Object> handle = Managed<InterpreterHandle>::New(isolate, cpp_handle);
   debug_info->set(kInterpreterHandleIndex, *handle);
   return cpp_handle->interpreter();

src/wasm/wasm-interpreter.h

@@ -20,7 +20,7 @@ class WasmInstanceObject;
 namespace wasm {
 
 // forward declarations.
-struct ModuleBytesEnv;
+struct ModuleWireBytes;
 struct WasmFunction;
 struct WasmModule;
 class WasmInterpreterInternals;
@@ -171,7 +171,9 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     uint32_t ActivationFrameBase(uint32_t activation_id);
   };
 
-  WasmInterpreter(Isolate* isolate, const ModuleBytesEnv& env);
+  WasmInterpreter(Isolate* isolate, const WasmModule* module,
+                  const ModuleWireBytes& wire_bytes, byte* globals_start,
+                  byte* mem_start, uint32_t mem_size);
   ~WasmInterpreter();
 
   //==========================================================================
@@ -197,12 +199,8 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
   Thread* GetThread(int id);
 
   //==========================================================================
-  // Memory access.
+  // Update the cached module env memory parameters after a grow memory event.
   //==========================================================================
-  size_t GetMemorySize();
-  WasmValue ReadMemory(size_t offset);
-  void WriteMemory(size_t offset, WasmValue val);
-  // Update the memory region, e.g. after external GrowMemory.
   void UpdateMemory(byte* mem_start, uint32_t mem_size);
 
   //==========================================================================
@@ -222,7 +220,7 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
 
  private:
   Zone zone_;
-  WasmInterpreterInternals* internals_;
+  WasmInterpreterInternals* const internals_;
 };
 
 }  // namespace wasm

src/wasm/wasm-objects.h

@@ -611,7 +611,7 @@ class WasmDebugInfo : public FixedArray {
   // WasmDebugInfo.
   // Use for testing only.
   V8_EXPORT_PRIVATE static wasm::WasmInterpreter* SetupForTesting(
-      Handle<WasmInstanceObject>, wasm::WasmInstance*);
+      Handle<WasmInstanceObject>);
 
   // Set a breakpoint in the given function at the given byte offset within that
   // function. This will redirect all future calls to this function to the

test/cctest/wasm/wasm-run-utils.h

@@ -69,9 +69,10 @@ using namespace v8::internal::wasm;
 
 const uint32_t kMaxGlobalsSize = 128;
 
-// A helper for module environments that adds the ability to allocate memory
-// and global variables. Contains a built-in {WasmModule} and
-// {WasmInstance}.
+// A buildable ModuleEnv. Globals are pre-set, however, memory and code may be
+// progressively added by a test. In turn, we piecemeal update the runtime
+// objects, i.e. {WasmInstanceObject}, {WasmCompiledModule} and, if necessary,
+// the interpreter.
 class TestingModule : public ModuleEnv {
  public:
   explicit TestingModule(Zone* zone, WasmExecutionMode mode = kExecuteCompiled)
@@ -89,8 +90,7 @@ class TestingModule : public ModuleEnv {
     memset(global_data, 0, sizeof(global_data));
     instance_object_ = InitInstanceObject();
     if (mode == kExecuteInterpreted) {
-      interpreter_ =
-          WasmDebugInfo::SetupForTesting(instance_object_, &instance_);
+      interpreter_ = WasmDebugInfo::SetupForTesting(instance_object_);
     }
   }
 
@@ -113,6 +113,21 @@ class TestingModule : public ModuleEnv {
     CHECK(size == 0 || instance->mem_start);
     memset(instance->mem_start, 0, size);
     instance->mem_size = size;
+    Handle<WasmCompiledModule> compiled_module =
+        handle(instance_object_->compiled_module());
+    Factory* factory = CcTest::i_isolate()->factory();
+    // It's not really necessary we recreate the Number objects,
+    // if we happened to have one, but this is a reasonable inefficiencly,
+    // given this is test.
+    WasmCompiledModule::recreate_embedded_mem_size(compiled_module, factory,
+                                                   instance->mem_size);
+    WasmCompiledModule::recreate_embedded_mem_start(
+        compiled_module, factory,
+        reinterpret_cast<size_t>(instance->mem_start));
+
+    if (interpreter_) {
+      interpreter_->UpdateMemory(instance->mem_start, instance->mem_size);
+    }
     return instance->mem_start;
   }
 
@@ -346,6 +361,12 @@ class TestingModule : public ModuleEnv {
     std::vector<Handle<FixedArray>> empty;
     Handle<WasmCompiledModule> compiled_module = WasmCompiledModule::New(
         isolate_, shared_module_data, code_table, empty, empty);
+    // This method is called when we initialize TestEnvironment. We don't
+    // have a memory yet, so we won't create it here. We'll update the
+    // interpreter when we get a memory. We do have globals, though.
+    WasmCompiledModule::recreate_globals_start(
+        compiled_module, isolate_->factory(),
+        reinterpret_cast<size_t>(instance->globals_start));
     Handle<FixedArray> weak_exported = isolate_->factory()->NewFixedArray(0);
     compiled_module->set_weak_exported_functions(weak_exported);
     DCHECK(WasmCompiledModule::IsWasmCompiledModule(*compiled_module));

test/common/wasm/wasm-module-runner.cc

@@ -91,8 +91,7 @@ int32_t InterpretWasmModule(Isolate* isolate,
   Zone zone(isolate->allocator(), ZONE_NAME);
   v8::internal::HandleScope scope(isolate);
 
-  WasmInterpreter* interpreter =
-      WasmDebugInfo::SetupForTesting(instance, nullptr);
+  WasmInterpreter* interpreter = WasmDebugInfo::SetupForTesting(instance);
   WasmInterpreter::HeapObjectsScope heap_objects_scope(interpreter, instance);
   WasmInterpreter::Thread* thread = interpreter->GetThread(0);
   thread->Reset();