[GC] Fix minor binary format issues of ordering and immediates (#3472)

Noticed by comparing to V8 and Wasp. After this things are almost identical,
but there is also at least https://bugs.chromium.org/p/v8/issues/detail?id=11300

Test updates are due to having an instruction with two operands of which one is
unreachable. The new order puts the non-unreachable first, so it is not removed by
round-tripping through the binary format like before (which removes all unreachable
code).

3 files changed, 26 insertions, 29 deletions

diff --git a/src/wasm-delegations-fields.h b/src/wasm-delegations-fields.h
index 8322fdeed..37fb60ef2 100644
--- a/src/wasm-delegations-fields.h
+++ b/src/wasm-delegations-fields.h
@@ -568,15 +568,15 @@ switch (DELEGATE_ID) {
   }
   case Expression::Id::RefTestId: {
     DELEGATE_START(RefTest);
-    DELEGATE_FIELD_CHILD(RefTest, ref);
     DELEGATE_FIELD_CHILD(RefTest, rtt);
+    DELEGATE_FIELD_CHILD(RefTest, ref);
     DELEGATE_END(RefTest);
     break;
   }
   case Expression::Id::RefCastId: {
     DELEGATE_START(RefCast);
-    DELEGATE_FIELD_CHILD(RefCast, ref);
     DELEGATE_FIELD_CHILD(RefCast, rtt);
+    DELEGATE_FIELD_CHILD(RefCast, ref);
     DELEGATE_END(RefCast);
     break;
   }
@@ -584,8 +584,8 @@ switch (DELEGATE_ID) {
     DELEGATE_START(BrOnCast);
     DELEGATE_FIELD_SCOPE_NAME_USE(BrOnCast, name);
     DELEGATE_FIELD_TYPE(BrOnCast, castType);
-    DELEGATE_FIELD_CHILD(BrOnCast, ref);
     DELEGATE_FIELD_CHILD(BrOnCast, rtt);
+    DELEGATE_FIELD_CHILD(BrOnCast, ref);
     DELEGATE_END(BrOnCast);
     break;
   }
@@ -618,8 +618,8 @@ switch (DELEGATE_ID) {
   case Expression::Id::StructSetId: {
     DELEGATE_START(StructSet);
     DELEGATE_FIELD_INT(StructSet, index);
-    DELEGATE_FIELD_CHILD(StructSet, ref);
     DELEGATE_FIELD_CHILD(StructSet, value);
+    DELEGATE_FIELD_CHILD(StructSet, ref);
     DELEGATE_END(StructSet);
     break;
   }
@@ -633,17 +633,17 @@ switch (DELEGATE_ID) {
   }
   case Expression::Id::ArrayGetId: {
     DELEGATE_START(ArrayGet);
-    DELEGATE_FIELD_CHILD(ArrayGet, ref);
     DELEGATE_FIELD_CHILD(ArrayGet, index);
+    DELEGATE_FIELD_CHILD(ArrayGet, ref);
     DELEGATE_FIELD_INT(ArrayGet, signed_);
     DELEGATE_END(ArrayGet);
     break;
   }
   case Expression::Id::ArraySetId: {
     DELEGATE_START(ArraySet);
-    DELEGATE_FIELD_CHILD(ArrayGet, ref);
-    DELEGATE_FIELD_CHILD(ArrayGet, index);
     DELEGATE_FIELD_CHILD(ArrayGet, value);
+    DELEGATE_FIELD_CHILD(ArrayGet, index);
+    DELEGATE_FIELD_CHILD(ArrayGet, ref);
     DELEGATE_END(ArraySet);
     break;
   }
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index bcf7c15fe..beaed1b11 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -5682,10 +5682,10 @@ bool WasmBinaryBuilder::maybeVisitRefTest(Expression*& out, uint32_t code) {
   }
   auto heapType1 = getHeapType();
   auto heapType2 = getHeapType();
-  auto* ref = popNonVoidExpression();
-  validateHeapTypeUsingChild(ref, heapType1);
   auto* rtt = popNonVoidExpression();
   validateHeapTypeUsingChild(rtt, heapType2);
+  auto* ref = popNonVoidExpression();
+  validateHeapTypeUsingChild(ref, heapType1);
   out = Builder(wasm).makeRefTest(ref, rtt);
   return true;
 }
@@ -5696,10 +5696,10 @@ bool WasmBinaryBuilder::maybeVisitRefCast(Expression*& out, uint32_t code) {
   }
   auto heapType1 = getHeapType();
   auto heapType2 = getHeapType();
-  auto* ref = popNonVoidExpression();
-  validateHeapTypeUsingChild(ref, heapType1);
   auto* rtt = popNonVoidExpression();
   validateHeapTypeUsingChild(rtt, heapType2);
+  auto* ref = popNonVoidExpression();
+  validateHeapTypeUsingChild(ref, heapType1);
   out = Builder(wasm).makeRefCast(ref, rtt);
   return true;
 }
@@ -5709,13 +5709,14 @@ bool WasmBinaryBuilder::maybeVisitBrOnCast(Expression*& out, uint32_t code) {
     return false;
   }
   auto name = getBreakTarget(getU32LEB()).name;
-  auto heapType1 = getHeapType();
-  auto heapType2 = getHeapType();
-  auto* ref = popNonVoidExpression();
-  validateHeapTypeUsingChild(ref, heapType1);
+  // TODO the spec has two heaptype immediates, but the V8 prototype does not;
+  //      match V8 for now.
   auto* rtt = popNonVoidExpression();
-  validateHeapTypeUsingChild(rtt, heapType2);
-  out = Builder(wasm).makeBrOnCast(name, heapType2, ref, rtt);
+  if (!rtt->type.isRtt()) {
+    throwError("bad rtt for br_on_cast");
+  }
+  auto* ref = popNonVoidExpression();
+  out = Builder(wasm).makeBrOnCast(name, rtt->type.getHeapType(), ref, rtt);
   return true;
 }
 
@@ -5732,11 +5733,11 @@ bool WasmBinaryBuilder::maybeVisitRttSub(Expression*& out, uint32_t code) {
   if (code != BinaryConsts::RttSub) {
     return false;
   }
-  // FIXME: the binary format may also have an extra heap type and index that
-  //        are not needed
-  auto heapType = getHeapType();
+  // TODO the spec has two heaptype immediates, but the V8 prototype does not;
+  //      match that for now.
+  auto targetHeapType = getHeapType();
   auto* parent = popNonVoidExpression();
-  out = Builder(wasm).makeRttSub(heapType, parent);
+  out = Builder(wasm).makeRttSub(targetHeapType, parent);
   return true;
 }
 
@@ -5793,9 +5794,9 @@ bool WasmBinaryBuilder::maybeVisitStructSet(Expression*& out, uint32_t code) {
   auto* curr = allocator.alloc<StructSet>();
   auto heapType = getHeapType();
   curr->index = getU32LEB();
+  curr->value = popNonVoidExpression();
   curr->ref = popNonVoidExpression();
   validateHeapTypeUsingChild(curr->ref, heapType);
-  curr->value = popNonVoidExpression();
   curr->finalize();
   out = curr;
   return true;
@@ -5831,9 +5832,9 @@ bool WasmBinaryBuilder::maybeVisitArrayGet(Expression*& out, uint32_t code) {
       return false;
   }
   auto heapType = getHeapType();
+  auto* index = popNonVoidExpression();
   auto* ref = popNonVoidExpression();
   validateHeapTypeUsingChild(ref, heapType);
-  auto* index = popNonVoidExpression();
   out = Builder(wasm).makeArrayGet(ref, index, signed_);
   return true;
 }
@@ -5843,10 +5844,10 @@ bool WasmBinaryBuilder::maybeVisitArraySet(Expression*& out, uint32_t code) {
     return false;
   }
   auto heapType = getHeapType();
+  auto* value = popNonVoidExpression();
+  auto* index = popNonVoidExpression();
   auto* ref = popNonVoidExpression();
   validateHeapTypeUsingChild(ref, heapType);
-  auto* index = popNonVoidExpression();
-  auto* value = popNonVoidExpression();
   out = Builder(wasm).makeArraySet(ref, index, value);
   return true;
 }
diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp
index b31aaf4d4..049573fc5 100644
--- a/src/wasm/wasm-stack.cpp
+++ b/src/wasm/wasm-stack.cpp
@@ -1952,8 +1952,6 @@ void BinaryInstWriter::visitRefCast(RefCast* curr) {
 void BinaryInstWriter::visitBrOnCast(BrOnCast* curr) {
   o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::BrOnCast)
     << U32LEB(getBreakIndex(curr->name));
-  parent.writeHeapType(curr->ref->type.getHeapType());
-  parent.writeHeapType(curr->getCastType().getHeapType());
 }
 
 void BinaryInstWriter::visitRttCanon(RttCanon* curr) {
@@ -1963,8 +1961,6 @@ void BinaryInstWriter::visitRttCanon(RttCanon* curr) {
 
 void BinaryInstWriter::visitRttSub(RttSub* curr) {
   o << int8_t(BinaryConsts::GCPrefix) << U32LEB(BinaryConsts::RttSub);
-  // FIXME: the binary format may also have an extra heap type and index that
-  //        are not needed
   parent.writeHeapType(curr->type.getRtt().heapType);
 }