Refactor stack writing code into a new StackWriter class (#1620)

This separates out the WasmBinaryWriter parts that do stack writing into a separate class, StackWriter. Previously the WasmBinaryWriter did both the general writing and the stack stuff, and the stack stuff has global state, which it manually cleaned up etc. - seems nicer to have it as a separate class, a class focused on just that one thing.

Should be no functional changes in this PR.

Also add a timeout to the wasm-reduce test, which happened to fail on one of the commits here. It was running slower on that commit for some reason, could have been random - I verified that general wasm writing speed is unaffected by this PR. (But I added the timeout to prevent future random timeouts.)

Author: kripken

check.py

@@ -283,7 +283,7 @@ def run_wasm_reduce_tests():
       t = os.path.join(test_dir, t)
       # convert to wasm
       run_command(WASM_AS + [t, '-o', 'a.wasm'])
-      run_command(WASM_REDUCE + ['a.wasm', '--command=%s b.wasm --fuzz-exec' % WASM_OPT[0], '-t', 'b.wasm', '-w', 'c.wasm'])
+      run_command(WASM_REDUCE + ['a.wasm', '--command=%s b.wasm --fuzz-exec' % WASM_OPT[0], '-t', 'b.wasm', '-w', 'c.wasm', '--timeout=4'])
       expected = t + '.txt'
       run_command(WASM_DIS + ['c.wasm', '-o', 'a.wast'])
       with open('a.wast') as seen:

src/wasm-binary.h

@@ -652,10 +652,79 @@ inline S32LEB binaryType(Type type) {
   return S32LEB(ret);
 }
 
-class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
+class WasmBinaryWriter;
+
+// Writes out binary format stack machine code for a Binaryen IR expression
+
+class StackWriter : public Visitor<StackWriter> {
+public:
+  // Without a function (offset for a global thing, etc.)
+  StackWriter(WasmBinaryWriter& parent, BufferWithRandomAccess& o, bool debug=false)
+    : func(nullptr), parent(parent), o(o), sourceMap(false), debug(debug) {}
+
+  // With a function - one is created for the entire function
+  StackWriter(Function* func, WasmBinaryWriter& parent, BufferWithRandomAccess& o, bool sourceMap=false, bool debug=false)
+    : func(func), parent(parent), o(o), sourceMap(sourceMap), debug(debug) {
+    mapLocals();
+  }
+
+  std::map<Type, size_t> numLocalsByType; // type => number of locals of that type in the compact form
+
+  // visits a node, emitting the proper code for it
+  void visit(Expression* curr);
+  // emits a node, but if it is a block with no name, emit a list of its contents
+  void visitPossibleBlockContents(Expression* curr);
+
+  void visitBlock(Block *curr);
+  void visitIf(If *curr);
+  void visitLoop(Loop *curr);
+  void visitBreak(Break *curr);
+  void visitSwitch(Switch *curr);
+  void visitCall(Call *curr);
+  void visitCallImport(CallImport *curr);
+  void visitCallIndirect(CallIndirect *curr);
+  void visitGetLocal(GetLocal *curr);
+  void visitSetLocal(SetLocal *curr);
+  void visitGetGlobal(GetGlobal *curr);
+  void visitSetGlobal(SetGlobal *curr);
+  void visitLoad(Load *curr);
+  void visitStore(Store *curr);
+  void visitAtomicRMW(AtomicRMW *curr);
+  void visitAtomicCmpxchg(AtomicCmpxchg *curr);
+  void visitAtomicWait(AtomicWait *curr);
+  void visitAtomicWake(AtomicWake *curr);
+  void visitConst(Const *curr);
+  void visitUnary(Unary *curr);
+  void visitBinary(Binary *curr);
+  void visitSelect(Select *curr);
+  void visitReturn(Return *curr);
+  void visitHost(Host *curr);
+  void visitNop(Nop *curr);
+  void visitUnreachable(Unreachable *curr);
+  void visitDrop(Drop *curr);
+
+private:
+  Function* func;
+  WasmBinaryWriter& parent;
+  BufferWithRandomAccess& o;
+  bool sourceMap;
+  bool debug;
+
+  std::map<Index, size_t> mappedLocals; // local index => index in compact form of [all int32s][all int64s]etc
+
+  std::vector<Name> breakStack;
+
+  int32_t getBreakIndex(Name name);
+  void emitMemoryAccess(size_t alignment, size_t bytes, uint32_t offset);
+
+  void mapLocals();
+};
+
+// Writes out wasm to the binary format
+
+class WasmBinaryWriter {
   Module* wasm;
   BufferWithRandomAccess& o;
-  Function* currFunction = nullptr;
   bool debug;
   bool debugInfo = true;
   std::ostream* sourceMap = nullptr;
@@ -664,6 +733,9 @@ class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
 
   MixedArena allocator;
 
+  Function::DebugLocation lastDebugLocation;
+  size_t lastBytecodeOffset;
+
   void prepare();
 public:
   WasmBinaryWriter(Module* input, BufferWithRandomAccess& o, bool debug = false) : wasm(input), o(o), debug(debug) {
@@ -703,10 +775,6 @@ class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
   int32_t getFunctionTypeIndex(Name type);
   void writeImports();
 
-  std::map<Index, size_t> mappedLocals; // local index => index in compact form of [all int32s][all int64s]etc
-  std::map<Type, size_t> numLocalsByType; // type => number of locals of that type in the compact form
-
-  void mapLocals(Function* function);
   void writeFunctionSignatures();
   void writeExpression(Expression* curr);
   void writeFunctions();
@@ -728,7 +796,7 @@ class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
 
   void writeSourceMapProlog();
   void writeSourceMapEpilog();
-  void writeDebugLocation(size_t offset, const Function::DebugLocation& loc);
+  void writeDebugLocation(Expression* curr, Function* func);
 
   // helpers
   void writeInlineString(const char* name);
@@ -746,58 +814,6 @@ class WasmBinaryWriter : public Visitor<WasmBinaryWriter, void> {
   void emitBuffer(const char* data, size_t size);
   void emitString(const char *str);
   void finishUp();
-
-  // AST writing via visitors
-  int depth = 0; // only for debugging
-
-  void recurse(Expression* curr);
-  std::vector<Name> breakStack;
-  Function::DebugLocation lastDebugLocation;
-  size_t lastBytecodeOffset;
-
-  void visit(Expression* curr) {
-    if (sourceMap && currFunction) {
-      // Dump the sourceMap debug info
-      auto& debugLocations = currFunction->debugLocations;
-      auto iter = debugLocations.find(curr);
-      if (iter != debugLocations.end() && iter->second != lastDebugLocation) {
-        writeDebugLocation(o.size(), iter->second);
-      }
-    }
-    Visitor<WasmBinaryWriter>::visit(curr);
-  }
-
-  void visitBlock(Block *curr);
-  // emits a node, but if it is a block with no name, emit a list of its contents
-  void recursePossibleBlockContents(Expression* curr);
-  void visitIf(If *curr);
-  void visitLoop(Loop *curr);
-  int32_t getBreakIndex(Name name);
-  void visitBreak(Break *curr);
-  void visitSwitch(Switch *curr);
-  void visitCall(Call *curr);
-  void visitCallImport(CallImport *curr);
-  void visitCallIndirect(CallIndirect *curr);
-  void visitGetLocal(GetLocal *curr);
-  void visitSetLocal(SetLocal *curr);
-  void visitGetGlobal(GetGlobal *curr);
-  void visitSetGlobal(SetGlobal *curr);
-  void emitMemoryAccess(size_t alignment, size_t bytes, uint32_t offset);
-  void visitLoad(Load *curr);
-  void visitStore(Store *curr);
-  void visitAtomicRMW(AtomicRMW *curr);
-  void visitAtomicCmpxchg(AtomicCmpxchg *curr);
-  void visitAtomicWait(AtomicWait *curr);
-  void visitAtomicWake(AtomicWake *curr);
-  void visitConst(Const *curr);
-  void visitUnary(Unary *curr);
-  void visitBinary(Binary *curr);
-  void visitSelect(Select *curr);
-  void visitReturn(Return *curr);
-  void visitHost(Host *curr);
-  void visitNop(Nop *curr);
-  void visitUnreachable(Unreachable *curr);
-  void visitDrop(Drop *curr);
 };
 
 class WasmBinaryBuilder {