Minimal Push/Pop support (#2207)

This is the first stage of adding support for stacky/multivaluey things. It adds new push/pop instructions, and so far just shows that they can be read and written, and that the optimizer doesn't do anything immediately wrong on them.

No fuzzer support, since there isn't a "correct" way to use these yet. The current test shows some "incorrect" usages of them, which is nice to see that we can parse/emit them, but we should replace them with proper usages of push/pop once we actually have those (see comments in the tests).

This should be enough to unblock exceptions (which needs a pop in try-catches). It is also a step towards multivalue (I added some docs about that), but most of multivalue is left to be done.

Author: kripken

README.md

@@ -67,6 +67,11 @@ There are a few differences between Binaryen IR and the WebAssembly language:
      WebAssembly's official text format is primarily a linear instruction list
      (with s-expression extensions). Binaryen can't read the linear style, but
      it can read a wasm text file if it contains only s-expressions.
+   * Binaryen uses Stack IR to optimize "stacky" code (that can't be
+     represented in structured form).
+   * In rare cases stacky code must be represented in Binaryen IR as well, like
+     popping a value in an exception catch. To support that Binaryen IR has
+     `push` and `pop` instructions.
  * Types and unreachable code
    * WebAssembly limits block/if/loop types to none and the concrete value types
      (i32, i64, f32, f64). Binaryen IR has an unreachable type, and it allows
@@ -103,6 +108,19 @@ There are a few differences between Binaryen IR and the WebAssembly language:
      emitted when generating wasm. Instead its list of operands will be directly
      used in the containing node. Such a block is sometimes called an "implicit
      block".
+ * Multivalue
+   * Binaryen will not represent multivalue instructions and values directly.
+     Binaryen's main focus is on optimization of wasm, and therefore the question
+     of whether we should have multivalue in the main IR is whether it justifes
+     the extra complexity there. Experiments show that the shrinking of code
+     size thanks to multivalue is useful but small, just 1-3% or so. Given that,
+     we prefer to keep the main IR simple, and focus on multivalue optimizations
+     in Stack IR, which is more suitable for such things.
+   * Binaryen does still need to implement the "ABI" level of multivalue, that
+     is, we need multivalue calls because those may cross module boundaries,
+     and so they are observable externally. To support that, Binaryen may use
+     `push` and `pop` as mentioned earlier; another option is to add LLVM-like
+     `extractvalue/composevalue` instructions.
 
 As a result, you might notice that round-trip conversions (wasm => Binaryen IR
 => wasm) change code a little in some corner cases.

scripts/gen-s-parser.py

@@ -41,6 +41,11 @@
     ("data.drop",      "makeDataDrop(s)"),
     ("memory.copy",    "makeMemoryCopy(s)"),
     ("memory.fill",    "makeMemoryFill(s)"),
+    ("push",           "makePush(s)"),
+    ("i32.pop",        "makePop(i32)"),
+    ("i64.pop",        "makePop(i64)"),
+    ("f32.pop",        "makePop(f32)"),
+    ("f64.pop",        "makePop(f64)"),
     ("i32.load",       "makeLoad(s, i32, /*isAtomic=*/false)"),
     ("i64.load",       "makeLoad(s, i64, /*isAtomic=*/false)"),
     ("f32.load",       "makeLoad(s, f32, /*isAtomic=*/false)"),

src/gen-s-parser.inc

@@ -200,6 +200,9 @@ switch (op[0]) {
                   default: goto parse_error;
                 }
               }
+              case 'p':
+                if (strcmp(op, "f32.pop") == 0) { return makePop(f32); }
+                goto parse_error;
               case 'r':
                 if (strcmp(op, "f32.reinterpret_i32") == 0) { return makeUnary(s, UnaryOp::ReinterpretInt32); }
                 goto parse_error;
@@ -459,9 +462,17 @@ switch (op[0]) {
                   default: goto parse_error;
                 }
               }
-              case 'p':
-                if (strcmp(op, "f64.promote_f32") == 0) { return makeUnary(s, UnaryOp::PromoteFloat32); }
-                goto parse_error;
+              case 'p': {
+                switch (op[5]) {
+                  case 'o':
+                    if (strcmp(op, "f64.pop") == 0) { return makePop(f64); }
+                    goto parse_error;
+                  case 'r':
+                    if (strcmp(op, "f64.promote_f32") == 0) { return makeUnary(s, UnaryOp::PromoteFloat32); }
+                    goto parse_error;
+                  default: goto parse_error;
+                }
+              }
               case 'r':
                 if (strcmp(op, "f64.reinterpret_i64") == 0) { return makeUnary(s, UnaryOp::ReinterpretInt64); }
                 goto parse_error;
@@ -1089,9 +1100,17 @@ switch (op[0]) {
               case 'o':
                 if (strcmp(op, "i32.or") == 0) { return makeBinary(s, BinaryOp::OrInt32); }
                 goto parse_error;
-              case 'p':
-                if (strcmp(op, "i32.popcnt") == 0) { return makeUnary(s, UnaryOp::PopcntInt32); }
-                goto parse_error;
+              case 'p': {
+                switch (op[7]) {
+                  case '\0':
+                    if (strcmp(op, "i32.pop") == 0) { return makePop(i32); }
+                    goto parse_error;
+                  case 'c':
+                    if (strcmp(op, "i32.popcnt") == 0) { return makeUnary(s, UnaryOp::PopcntInt32); }
+                    goto parse_error;
+                  default: goto parse_error;
+                }
+              }
               case 'r': {
                 switch (op[5]) {
                   case 'e': {
@@ -1757,9 +1776,17 @@ switch (op[0]) {
               case 'o':
                 if (strcmp(op, "i64.or") == 0) { return makeBinary(s, BinaryOp::OrInt64); }
                 goto parse_error;
-              case 'p':
-                if (strcmp(op, "i64.popcnt") == 0) { return makeUnary(s, UnaryOp::PopcntInt64); }
-                goto parse_error;
+              case 'p': {
+                switch (op[7]) {
+                  case '\0':
+                    if (strcmp(op, "i64.pop") == 0) { return makePop(i64); }
+                    goto parse_error;
+                  case 'c':
+                    if (strcmp(op, "i64.popcnt") == 0) { return makeUnary(s, UnaryOp::PopcntInt64); }
+                    goto parse_error;
+                  default: goto parse_error;
+                }
+              }
               case 'r': {
                 switch (op[5]) {
                   case 'e': {
@@ -2198,6 +2225,9 @@ switch (op[0]) {
   case 'n':
     if (strcmp(op, "nop") == 0) { return makeNop(); }
     goto parse_error;
+  case 'p':
+    if (strcmp(op, "push") == 0) { return makePush(s); }
+    goto parse_error;
   case 'r':
     if (strcmp(op, "return") == 0) { return makeReturn(s); }
     goto parse_error;

src/ir/ExpressionAnalyzer.cpp

@@ -209,6 +209,8 @@ template<typename T> void visitImmediates(Expression* curr, T& visitor) {
     }
     void visitNop(Nop* curr) {}
     void visitUnreachable(Unreachable* curr) {}
+    void visitPush(Push* curr) {}
+    void visitPop(Pop* curr) {}
   } singleton(curr, visitor);
 }
 

src/ir/ReFinalize.cpp

@@ -157,6 +157,8 @@ void ReFinalize::visitReturn(Return* curr) { curr->finalize(); }
 void ReFinalize::visitHost(Host* curr) { curr->finalize(); }
 void ReFinalize::visitNop(Nop* curr) { curr->finalize(); }
 void ReFinalize::visitUnreachable(Unreachable* curr) { curr->finalize(); }
+void ReFinalize::visitPush(Push* curr) { curr->finalize(); }
+void ReFinalize::visitPop(Pop* curr) { curr->finalize(); }
 
 void ReFinalize::visitFunction(Function* curr) {
   // we may have changed the body from unreachable to none, which might be bad

src/ir/effects.h

@@ -368,6 +368,8 @@ struct EffectAnalyzer
   }
   void visitNop(Nop* curr) {}
   void visitUnreachable(Unreachable* curr) { branches = true; }
+  void visitPush(Push* curr) { calls = true; }
+  void visitPop(Pop* curr) { calls = true; }
 
   // Helpers
 

src/ir/utils.h

@@ -146,6 +146,8 @@ struct ReFinalize
   void visitHost(Host* curr);
   void visitNop(Nop* curr);
   void visitUnreachable(Unreachable* curr);
+  void visitPush(Push* curr);
+  void visitPop(Pop* curr);
 
   void visitFunction(Function* curr);
 
@@ -203,6 +205,8 @@ struct ReFinalizeNode : public OverriddenVisitor<ReFinalizeNode> {
   void visitHost(Host* curr) { curr->finalize(); }
   void visitNop(Nop* curr) { curr->finalize(); }
   void visitUnreachable(Unreachable* curr) { curr->finalize(); }
+  void visitPush(Push* curr) { curr->finalize(); }
+  void visitPop(Pop* curr) { curr->finalize(); }
 
   void visitFunctionType(FunctionType* curr) { WASM_UNREACHABLE(); }
   void visitExport(Export* curr) { WASM_UNREACHABLE(); }

src/passes/DeadCodeElimination.cpp

@@ -308,6 +308,10 @@ struct DeadCodeElimination
           DELEGATE(MemoryCopy);
         case Expression::Id::MemoryFillId:
           DELEGATE(MemoryFill);
+        case Expression::Id::PushId:
+          DELEGATE(Push);
+        case Expression::Id::PopId:
+          DELEGATE(Pop);
         case Expression::Id::InvalidId:
           WASM_UNREACHABLE();
         case Expression::Id::NumExpressionIds:

src/passes/Precompute.cpp

@@ -130,6 +130,8 @@ class PrecomputingExpressionRunner
   Flow visitMemoryCopy(MemoryCopy* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
   Flow visitMemoryFill(MemoryFill* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
   Flow visitHost(Host* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
+  Flow visitPush(Push* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
+  Flow visitPop(Pop* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
 
   void trap(const char* why) override { throw NonstandaloneException(); }
 };

src/passes/Print.cpp

@@ -57,7 +57,8 @@ static Name printableLocal(Index index, Function* func) {
 
 // Prints the internal contents of an expression: everything but
 // the children.
-struct PrintExpressionContents : public Visitor<PrintExpressionContents> {
+struct PrintExpressionContents
+  : public OverriddenVisitor<PrintExpressionContents> {
   Function* currFunction = nullptr;
   std::ostream& o;
 
@@ -1150,11 +1151,17 @@ struct PrintExpressionContents : public Visitor<PrintExpressionContents> {
   }
   void visitNop(Nop* curr) { printMinor(o, "nop"); }
   void visitUnreachable(Unreachable* curr) { printMinor(o, "unreachable"); }
+  void visitPush(Push* curr) { prepareColor(o) << "push"; }
+  void visitPop(Pop* curr) {
+    prepareColor(o) << printType(curr->type);
+    o << ".pop";
+    restoreNormalColor(o);
+  }
 };
 
 // Prints an expression in s-expr format, including both the
 // internal contents and the nested children.
-struct PrintSExpression : public Visitor<PrintSExpression> {
+struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
   std::ostream& o;
   unsigned indent = 0;
 
@@ -1205,7 +1212,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
 
   void visit(Expression* curr) {
     printDebugLocation(curr);
-    Visitor<PrintSExpression>::visit(curr);
+    OverriddenVisitor<PrintSExpression>::visit(curr);
   }
 
   void setMinify(bool minify_) {
@@ -1621,6 +1628,18 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
     PrintExpressionContents(currFunction, o).visit(curr);
     o << ')';
   }
+  void visitPush(Push* curr) {
+    o << '(';
+    PrintExpressionContents(currFunction, o).visit(curr);
+    incIndent();
+    printFullLine(curr->value);
+    decIndent();
+  }
+  void visitPop(Pop* curr) {
+    o << '(';
+    PrintExpressionContents(currFunction, o).visit(curr);
+    o << ')';
+  }
   // Module-level visitors
   void visitFunctionType(FunctionType* curr, Name* internalName = nullptr) {
     o << "(func";