[wasm] Use pimpl idiom for interpreter thread implementation

We used dynamic dispatch before, which has considerable runtime
overhead. Pimpl (a.k.a. opaque pointer) has the same memory overhead
of one additional pointer, but less runtime overhead.

R=titzer@chromium.org

Review-Url: https://codereview.chromium.org/2639013002
Cr-Commit-Position: refs/heads/master@{#42442}

src/wasm/wasm-interpreter.cc

@@ -919,7 +919,7 @@ class CodeMap {
 };
 
 // Responsible for executing code directly.
-class ThreadImpl : public WasmInterpreter::Thread {
+class WasmInterpreter::ThreadImpl : public ZoneObject {
  public:
   ThreadImpl(Zone* zone, CodeMap* codemap, WasmInstance* instance)
       : codemap_(codemap),
@@ -932,15 +932,15 @@ class ThreadImpl : public WasmInterpreter::Thread {
         trap_reason_(kTrapCount),
         possible_nondeterminism_(false) {}
 
-  virtual ~ThreadImpl() {}
+  ~ThreadImpl() {}
 
   //==========================================================================
   // Implementation of public interface for WasmInterpreter::Thread.
   //==========================================================================
 
-  virtual WasmInterpreter::State state() { return state_; }
+  WasmInterpreter::State state() { return state_; }
 
-  virtual void PushFrame(const WasmFunction* function, WasmVal* args) {
+  void PushFrame(const WasmFunction* function, WasmVal* args) {
     InterpreterCode* code = codemap()->FindCode(function);
     CHECK_NOT_NULL(code);
     frames_.push_back({code, 0, 0, stack_.size()});
@@ -955,7 +955,7 @@ class ThreadImpl : public WasmInterpreter::Thread {
           frames_.back().ret_pc);
   }
 
-  virtual WasmInterpreter::State Run() {
+  WasmInterpreter::State Run() {
     do {
       TRACE("  => Run()\n");
       if (state_ == WasmInterpreter::STOPPED ||
@@ -967,7 +967,7 @@ class ThreadImpl : public WasmInterpreter::Thread {
     return state_;
   }
 
-  virtual WasmInterpreter::State Step() {
+  WasmInterpreter::State Step() {
     TRACE("  => Step()\n");
     if (state_ == WasmInterpreter::STOPPED ||
         state_ == WasmInterpreter::PAUSED) {
@@ -977,9 +977,9 @@ class ThreadImpl : public WasmInterpreter::Thread {
     return state_;
   }
 
-  virtual void Pause() { UNIMPLEMENTED(); }
+  void Pause() { UNIMPLEMENTED(); }
 
-  virtual void Reset() {
+  void Reset() {
     TRACE("----- RESET -----\n");
     stack_.clear();
     frames_.clear();
@@ -988,33 +988,28 @@ class ThreadImpl : public WasmInterpreter::Thread {
     possible_nondeterminism_ = false;
   }
 
-  virtual int GetFrameCount() { return static_cast<int>(frames_.size()); }
+  int GetFrameCount() { return static_cast<int>(frames_.size()); }
 
-  virtual const WasmFrame* GetFrame(int index) {
+  const WasmFrame* GetFrame(int index) {
     UNIMPLEMENTED();
     return nullptr;
   }
 
-  virtual WasmFrame* GetMutableFrame(int index) {
+  WasmFrame* GetMutableFrame(int index) {
     UNIMPLEMENTED();
     return nullptr;
   }
 
-  virtual WasmVal GetReturnValue(int index) {
+  WasmVal GetReturnValue(int index) {
     if (state_ == WasmInterpreter::TRAPPED) return WasmVal(0xdeadbeef);
     CHECK_EQ(WasmInterpreter::FINISHED, state_);
     CHECK_LT(static_cast<size_t>(index), stack_.size());
     return stack_[index];
   }
 
-  virtual pc_t GetBreakpointPc() { return break_pc_; }
+  pc_t GetBreakpointPc() { return break_pc_; }
 
-  virtual bool PossibleNondeterminism() { return possible_nondeterminism_; }
-
-  bool Terminated() {
-    return state_ == WasmInterpreter::TRAPPED ||
-           state_ == WasmInterpreter::FINISHED;
-  }
+  bool PossibleNondeterminism() { return possible_nondeterminism_; }
 
  private:
   // Entries on the stack of functions being evaluated.
@@ -1703,6 +1698,38 @@ class ThreadImpl : public WasmInterpreter::Thread {
   }
 };
 
+//============================================================================
+// Forwarding functions of WasmInterpreter::Thread (pimpl idiom).
+//============================================================================
+WasmInterpreter::Thread::Thread(ThreadImpl* impl) : impl_(impl) {}
+WasmInterpreter::State WasmInterpreter::Thread::state() {
+  return impl_->state();
+}
+void WasmInterpreter::Thread::PushFrame(const WasmFunction* function,
+                                        WasmVal* args) {
+  return impl_->PushFrame(function, args);
+}
+WasmInterpreter::State WasmInterpreter::Thread::Run() { return impl_->Run(); }
+WasmInterpreter::State WasmInterpreter::Thread::Step() { return impl_->Step(); }
+void WasmInterpreter::Thread::Pause() { return impl_->Pause(); }
+void WasmInterpreter::Thread::Reset() { return impl_->Reset(); }
+pc_t WasmInterpreter::Thread::GetBreakpointPc() {
+  return impl_->GetBreakpointPc();
+}
+int WasmInterpreter::Thread::GetFrameCount() { return impl_->GetFrameCount(); }
+const WasmFrame* WasmInterpreter::Thread::GetFrame(int index) {
+  return impl_->GetFrame(index);
+}
+WasmFrame* WasmInterpreter::Thread::GetMutableFrame(int index) {
+  return impl_->GetMutableFrame(index);
+}
+WasmVal WasmInterpreter::Thread::GetReturnValue(int index) {
+  return impl_->GetReturnValue(index);
+}
+bool WasmInterpreter::Thread::PossibleNondeterminism() {
+  return impl_->PossibleNondeterminism();
+}
+
 //============================================================================
 // The implementation details of the interpreter.
 //============================================================================
@@ -1713,7 +1740,7 @@ class WasmInterpreterInternals : public ZoneObject {
   // pointer might be invalidated after constructing the interpreter.
   const ZoneVector<uint8_t> module_bytes_;
   CodeMap codemap_;
-  ZoneVector<ThreadImpl*> threads_;
+  ZoneVector<WasmInterpreter::Thread> threads_;
 
   WasmInterpreterInternals(Zone* zone, const ModuleBytesEnv& env)
       : instance_(env.instance),
@@ -1721,14 +1748,11 @@ class WasmInterpreterInternals : public ZoneObject {
         codemap_(env.instance ? env.instance->module : nullptr,
                  module_bytes_.data(), zone),
         threads_(zone) {
-    threads_.push_back(new ThreadImpl(zone, &codemap_, env.instance));
+    threads_.push_back(WasmInterpreter::Thread(
+        new (zone) WasmInterpreter::ThreadImpl(zone, &codemap_, env.instance)));
   }
 
-  void Delete() {
-    // TODO(titzer): CFI doesn't like threads in the ZoneVector.
-    for (auto t : threads_) delete t;
-    threads_.resize(0);
-  }
+  void Delete() { threads_.clear(); }
 };
 
 //============================================================================
@@ -1741,9 +1765,9 @@ WasmInterpreter::WasmInterpreter(const ModuleBytesEnv& env,
 
 WasmInterpreter::~WasmInterpreter() { internals_->Delete(); }
 
-void WasmInterpreter::Run() { internals_->threads_[0]->Run(); }
+void WasmInterpreter::Run() { internals_->threads_[0].Run(); }
 
-void WasmInterpreter::Pause() { internals_->threads_[0]->Pause(); }
+void WasmInterpreter::Pause() { internals_->threads_[0].Pause(); }
 
 bool WasmInterpreter::SetBreakpoint(const WasmFunction* function, pc_t pc,
                                     bool enabled) {
@@ -1788,7 +1812,7 @@ int WasmInterpreter::GetThreadCount() {
 
 WasmInterpreter::Thread* WasmInterpreter::GetThread(int id) {
   CHECK_EQ(0, id);  // only one thread for now.
-  return internals_->threads_[id];
+  return &internals_->threads_[id];
 }
 
 WasmVal WasmInterpreter::GetLocalVal(const WasmFrame* frame, int index) {

src/wasm/wasm-interpreter.h

@@ -79,7 +79,12 @@ inline void WasmVal::to() {
 class WasmFrame {
  public:
   const WasmFunction* function() const { return function_; }
-  int pc() const { return pc_; }
+  int pc() const {
+    // TODO(wasm): Remove USE once we actually use them.
+    USE(fp_);
+    USE(sp_);
+    return pc_;
+  }
 
  private:
   friend class WasmInterpreter;
@@ -108,31 +113,36 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
   enum State { STOPPED, RUNNING, PAUSED, FINISHED, TRAPPED };
 
   // Representation of a thread in the interpreter.
-  class Thread {
+  class ThreadImpl;
+  class V8_EXPORT_PRIVATE Thread {
+    ThreadImpl* impl_;
+
    public:
+    explicit Thread(ThreadImpl*);
+
     // Execution control.
-    virtual State state() = 0;
-    virtual void PushFrame(const WasmFunction* function, WasmVal* args) = 0;
-    virtual State Run() = 0;
-    virtual State Step() = 0;
-    virtual void Pause() = 0;
-    virtual void Reset() = 0;
-    virtual ~Thread() {}
+    State state();
+    void PushFrame(const WasmFunction* function, WasmVal* args);
+    State Run();
+    State Step();
+    void Pause();
+    void Reset();
 
     // Stack inspection and modification.
-    virtual pc_t GetBreakpointPc() = 0;
-    virtual int GetFrameCount() = 0;
-    virtual const WasmFrame* GetFrame(int index) = 0;
-    virtual WasmFrame* GetMutableFrame(int index) = 0;
-    virtual WasmVal GetReturnValue(int index = 0) = 0;
+    pc_t GetBreakpointPc();
+    int GetFrameCount();
+    const WasmFrame* GetFrame(int index);
+    WasmFrame* GetMutableFrame(int index);
+    WasmVal GetReturnValue(int index = 0);
     // Returns true if the thread executed an instruction which may produce
     // nondeterministic results, e.g. float div, float sqrt, and float mul,
     // where the sign bit of a NaN is nondeterministic.
-    virtual bool PossibleNondeterminism() = 0;
+    bool PossibleNondeterminism();
 
     // Thread-specific breakpoints.
-    bool SetBreakpoint(const WasmFunction* function, int pc, bool enabled);
-    bool GetBreakpoint(const WasmFunction* function, int pc);
+    // TODO(wasm): Implement this once we support multiple threads.
+    // bool SetBreakpoint(const WasmFunction* function, int pc, bool enabled);
+    // bool GetBreakpoint(const WasmFunction* function, int pc);
   };
 
   WasmInterpreter(const ModuleBytesEnv& env, AccountingAllocator* allocator);