[GC] br_on_null (WebAssembly#3528)

This is only partial support, as br_on_null also has an extra optional value in the spec. Implementing that is cumbersome in binaryen, and there is ongoing spec discussions about it (see WebAssembly/function-references#45 ), so for now we only support the simple case without the default value. Also fix prefixed opcodes to be LEBs in RefAs, which was noticed here as the change here made it noticeable whether the values were int8 or LEBs.
tlively · Feb 1, 2021 · eafb0a4 · eafb0a4
1 parent 02f8c56
commit eafb0a4
Show file tree

Hide file tree

Showing 13 changed files with 94 additions and 21 deletions.
diff --git a/scripts/gen-s-parser.py b/scripts/gen-s-parser.py
@@ -549,6 +549,7 @@
     ("i31.get_u",            "makeI31Get(s, false)"),
     ("ref.test",             "makeRefTest(s)"),
     ("ref.cast",             "makeRefCast(s)"),
+    ("br_on_null",           "makeBrOn(s, BrOnNull)"),
     ("br_on_cast",           "makeBrOn(s, BrOnCast)"),
     ("br_on_func",           "makeBrOn(s, BrOnFunc)"),
     ("br_on_data",           "makeBrOn(s, BrOnData)"),

diff --git a/src/gen-s-parser.inc b/src/gen-s-parser.inc
@@ -85,6 +85,9 @@ switch (op[0]) {
                   case 'i':
                     if (strcmp(op, "br_on_i31") == 0) { return makeBrOn(s, BrOnI31); }
                     goto parse_error;
+                  case 'n':
+                    if (strcmp(op, "br_on_null") == 0) { return makeBrOn(s, BrOnNull); }
+                    goto parse_error;
                   default: goto parse_error;
                 }
               }

diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
@@ -1785,6 +1785,9 @@ struct PrintExpressionContents
   }
   void visitBrOn(BrOn* curr) {
     switch (curr->op) {
+      case BrOnNull:
+        printMedium(o, "br_on_null ");
+        break;
       case BrOnCast:
         printMedium(o, "br_on_cast ");
         break;

diff --git a/src/wasm-binary.h b/src/wasm-binary.h
@@ -997,6 +997,7 @@ enum ASTNodes {
   RefIsNull = 0xd1,
   RefFunc = 0xd2,
   RefAsNonNull = 0xd3,
+  BrOnNull = 0xd4,
 
   // exception handling opcodes
 

diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
@@ -1490,6 +1490,15 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
     }
     const auto& value = flow.getSingleValue();
     NOTE_EVAL1(value);
+    if (curr->op == BrOnNull) {
+      // Unlike the others, BrOnNull does not propagate the value if it takes
+      // the branch.
+      if (value.isNull()) {
+        return Flow(curr->name);
+      }
+      // If the branch is not taken, we return the non-null value.
+      return {value};
+    }
     if (value.isNull()) {
       return {value};
     }

diff --git a/src/wasm.h b/src/wasm.h
@@ -557,6 +557,7 @@ enum RefAsOp {
 };
 
 enum BrOnOp {
+  BrOnNull,
   BrOnCast,
   BrOnFunc,
   BrOnData,
@@ -1400,6 +1401,12 @@ class BrOn : public SpecificExpression<Expression::BrOnId> {
   // BrOnCast has an rtt that is used in the cast.
   Expression* rtt;
 
+  // TODO: BrOnNull also has an optional extra value in the spec, which we do
+  //       not support. See also the discussion on
+  //       https://github.com/WebAssembly/function-references/issues/45
+  //       - depending on the decision there, we may want to move BrOnNull into
+  //       Break or a new class of its own.
+
   void finalize();
 
   Type getCastType();

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
@@ -2879,6 +2879,9 @@ BinaryConsts::ASTNodes WasmBinaryBuilder::readExpression(Expression*& curr) {
     case BinaryConsts::RefAsNonNull:
       visitRefAs((curr = allocator.alloc<RefAs>())->cast<RefAs>(), code);
       break;
+    case BinaryConsts::BrOnNull:
+      maybeVisitBrOn(curr, code);
+      break;
     case BinaryConsts::Try:
       visitTryOrTryInBlock(curr);
       break;
@@ -5831,6 +5834,9 @@ bool WasmBinaryBuilder::maybeVisitRefCast(Expression*& out, uint32_t code) {
 bool WasmBinaryBuilder::maybeVisitBrOn(Expression*& out, uint32_t code) {
   BrOnOp op;
   switch (code) {
+    case BinaryConsts::BrOnNull:
+      op = BrOnNull;
+      break;
     case BinaryConsts::BrOnCast:
       op = BrOnCast;
       break;

diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp
@@ -1982,19 +1982,21 @@ void BinaryInstWriter::visitRefCast(RefCast* curr) {
 }
 
 void BinaryInstWriter::visitBrOn(BrOn* curr) {
-  o << int8_t(BinaryConsts::GCPrefix);
   switch (curr->op) {
+    case BrOnNull:
+      o << int8_t(BinaryConsts::BrOnNull);
+      break;
     case BrOnCast:
-      o << U32LEB(BinaryConsts::BrOnCast);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::BrOnCast);
       break;
     case BrOnFunc:
-      o << U32LEB(BinaryConsts::BrOnFunc);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::BrOnFunc);
       break;
     case BrOnData:
-      o << U32LEB(BinaryConsts::BrOnData);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::BrOnData);
       break;
     case BrOnI31:
-      o << U32LEB(BinaryConsts::BrOnI31);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::BrOnI31);
       break;
     default:
       WASM_UNREACHABLE("invalid br_on_*");
@@ -2085,13 +2087,13 @@ void BinaryInstWriter::visitRefAs(RefAs* curr) {
       o << int8_t(BinaryConsts::RefAsNonNull);
       break;
     case RefAsFunc:
-      o << int8_t(BinaryConsts::GCPrefix) << int8_t(BinaryConsts::RefAsFunc);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::RefAsFunc);
       break;
     case RefAsData:
-      o << int8_t(BinaryConsts::GCPrefix) << int8_t(BinaryConsts::RefAsData);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::RefAsData);
       break;
     case RefAsI31:
-      o << int8_t(BinaryConsts::GCPrefix) << int8_t(BinaryConsts::RefAsI31);
+      o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::RefAsI31);
       break;
     default:
       WASM_UNREACHABLE("invalid ref.as_*");

diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
@@ -936,13 +936,24 @@ void BrOn::finalize() {
       (rtt && rtt->type == Type::unreachable)) {
     type = Type::unreachable;
   } else {
-    type = ref->type;
+    if (op == BrOnNull) {
+      // If BrOnNull does not branch, it flows out the existing value as
+      // non-null.
+      // FIXME: When we support non-nullable types, this should be non-nullable.
+      type = Type(ref->type.getHeapType(), Nullable);
+    } else {
+      type = ref->type;
+    }
   }
 }
 
 Type BrOn::getCastType() {
   switch (op) {
+    case BrOnNull:
+      // BrOnNull does not send a value on the branch.
+      return Type::none;
     case BrOnCast:
+      // FIXME: When we support non-nullable types, this should be non-nullable.
       return Type(rtt->type.getHeapType(), Nullable);
     case BrOnFunc:
       return Type::funcref;

diff --git a/test/heap-types.wast b/test/heap-types.wast
@@ -195,6 +195,12 @@
   )
   (func $br_on_X (param $x anyref)
     (local $y anyref)
+    (local $z (ref any))
+    (block $null
+      (local.set $z
+        (br_on_null $null (local.get $x))
+      )
+    )
     (drop
       (block $func (result funcref)
         (local.set $y

diff --git a/test/heap-types.wast.from-wast b/test/heap-types.wast.from-wast
@@ -243,6 +243,14 @@
  )
  (func $br_on_X (param $x anyref)
   (local $y anyref)
+  (local $z anyref)
+  (block $null
+   (local.set $z
+    (br_on_null $null
+     (local.get $x)
+    )
+   )
+  )
   (drop
    (block $func (result funcref)
     (local.set $y

diff --git a/test/heap-types.wast.fromBinary b/test/heap-types.wast.fromBinary
@@ -242,30 +242,38 @@
  )
  (func $br_on_X (param $x anyref)
   (local $y anyref)
+  (local $z anyref)
+  (block $label$1
+   (local.set $z
+    (br_on_null $label$1
+     (local.get $x)
+    )
+   )
+  )
   (drop
-   (block $label$1 (result funcref)
+   (block $label$2 (result funcref)
     (local.set $y
-     (br_on_func $label$1
+     (br_on_func $label$2
       (local.get $x)
      )
     )
     (ref.null func)
    )
   )
   (drop
-   (block $label$2 (result (ref null data))
+   (block $label$3 (result (ref null data))
     (local.set $y
-     (br_on_data $label$2
+     (br_on_data $label$3
       (local.get $x)
      )
     )
     (ref.null data)
    )
   )
   (drop
-   (block $label$3 (result (ref null i31))
+   (block $label$4 (result (ref null i31))
     (local.set $y
-     (br_on_i31 $label$3
+     (br_on_i31 $label$4
       (local.get $x)
      )
     )

diff --git a/test/heap-types.wast.fromBinary.noDebugInfo b/test/heap-types.wast.fromBinary.noDebugInfo
@@ -242,30 +242,38 @@
  )
  (func $7 (param $0 anyref)
   (local $1 anyref)
+  (local $2 anyref)
+  (block $label$1
+   (local.set $2
+    (br_on_null $label$1
+     (local.get $0)
+    )
+   )
+  )
   (drop
-   (block $label$1 (result funcref)
+   (block $label$2 (result funcref)
     (local.set $1
-     (br_on_func $label$1
+     (br_on_func $label$2
       (local.get $0)
      )
     )
     (ref.null func)
    )
   )
   (drop
-   (block $label$2 (result (ref null data))
+   (block $label$3 (result (ref null data))
     (local.set $1
-     (br_on_data $label$2
+     (br_on_data $label$3
       (local.get $0)
      )
     )
     (ref.null data)
    )
   )
   (drop
-   (block $label$3 (result (ref null i31))
+   (block $label$4 (result (ref null i31))
     (local.set $1
-     (br_on_i31 $label$3
+     (br_on_i31 $label$4
       (local.get $0)
      )
     )