[wasm-gc] Change reference type encoding to match proposal spec

Bug: v8:7748
Change-Id: I9af885e4c33541a8e065082ae7f07804bd11807a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2252190
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68443}

src/wasm/function-body-decoder-impl.h

@@ -145,19 +145,10 @@ ValueType read_value_type(Decoder* decoder, const byte* pc,
   }
 
   ValueTypeCode code = static_cast<ValueTypeCode>(val);
-  switch (code) {
-    case kLocalI32:
-      return kWasmI32;
-    case kLocalI64:
-      return kWasmI64;
-    case kLocalF32:
-      return kWasmF32;
-    case kLocalF64:
-      return kWasmF64;
 
-#define REF_TYPE_CASE(heap_type, strict_str, nullable, feature)            \
-  case kLocal##heap_type##strict_str##Ref: {                               \
-    ValueType result = ValueType::Ref(kHeap##heap_type, k##nullable);      \
+#define REF_TYPE_CASE(heap_type, nullable, feature)                        \
+  case kLocal##heap_type##Ref: {                                           \
+    ValueType result = ValueType::Ref(kHeap##heap_type, nullable);         \
     if (enabled.has_##feature()) {                                         \
       return result;                                                       \
     }                                                                      \
@@ -167,40 +158,50 @@ ValueType read_value_type(Decoder* decoder, const byte* pc,
     return kWasmBottom;                                                    \
   }
 
-#define NULLABLE_CASE(heap_type, feature) \
-  REF_TYPE_CASE(heap_type, , Nullable, feature)
-#define NON_NULLABLE_CASE(heap_type, feature) \
-  REF_TYPE_CASE(heap_type, Strict, NonNullable, feature)
-
-      NULLABLE_CASE(Func, reftypes)
-      NULLABLE_CASE(Extern, reftypes)
-      NULLABLE_CASE(Eq, gc)
-      NULLABLE_CASE(Exn, eh)
-      NON_NULLABLE_CASE(Func, typed_funcref)
-      NON_NULLABLE_CASE(Extern, typed_funcref)
-      NON_NULLABLE_CASE(Eq, gc)
-      NON_NULLABLE_CASE(Exn, eh)
-#undef REF_TYPE_CASE
-#undef NULLABLE_CASE
-#undef NON_NULLABLE_CASE
+  switch (code) {
+    REF_TYPE_CASE(Func, kNullable, reftypes)
+    REF_TYPE_CASE(Extern, kNullable, reftypes)
+    REF_TYPE_CASE(Eq, kNullable, gc)
+    REF_TYPE_CASE(Exn, kNullable, eh)
+    case kLocalI32:
+      return kWasmI32;
+    case kLocalI64:
+      return kWasmI64;
+    case kLocalF32:
+      return kWasmF32;
+    case kLocalF64:
+      return kWasmF64;
+    case kLocalRef:
+    case kLocalOptRef: {
+      // Set length for the macro-defined cases:
+      *length += 1;
+      Nullability nullability = code == kLocalOptRef ? kNullable : kNonNullable;
+      uint8_t heap_index = decoder->read_u8<validate>(pc + 1, "heap type");
+      switch (static_cast<ValueTypeCode>(heap_index)) {
+        REF_TYPE_CASE(Func, nullability, typed_funcref)
+        REF_TYPE_CASE(Extern, nullability, typed_funcref)
+        REF_TYPE_CASE(Eq, nullability, gc)
+        REF_TYPE_CASE(Exn, nullability, eh)
+        default:
+          uint32_t type_index =
+              decoder->read_u32v<validate>(pc + 1, length, "type index");
+          *length += 1;
+          if (!enabled.has_gc()) {
+            decoder->error(
+                pc,
+                "invalid value type '(ref [null] (type $t))', enable with "
+                "--experimental-wasm-typed-gc");
+            return kWasmBottom;
+          }
 
-    case kLocalIndexedStrictRef:
-    case kLocalIndexedRef: {
-      Nullability nullability =
-          code == kLocalIndexedRef ? kNullable : kNonNullable;
-      if (enabled.has_gc()) {
-        uint32_t type_index =
-            decoder->read_u32v<validate>(pc + 1, length, "type index");
-        (*length)++;
-        if (!VALIDATE(type_index < kV8MaxWasmTypes)) {
-          decoder->errorf(pc,
-                          "Type index %u is greater than the maximum "
-                          "number %zu of type definitions supported by V8",
-                          type_index, kV8MaxWasmTypes);
-          return kWasmBottom;
-        } else {
+          if (!VALIDATE(type_index < kV8MaxWasmTypes)) {
+            decoder->errorf(pc + 1,
+                            "Type index %u is greater than the maximum "
+                            "number %zu of type definitions supported by V8",
+                            type_index, kV8MaxWasmTypes);
+            return kWasmBottom;
+          }
           return ValueType::Ref(static_cast<HeapType>(type_index), nullability);
-        }
       }
       decoder->errorf(
           pc,
@@ -208,6 +209,7 @@ ValueType read_value_type(Decoder* decoder, const byte* pc,
           nullability ? " null" : "");
       return kWasmBottom;
     }
+#undef REF_TYPE_CASE
     case kLocalRtt:
       if (enabled.has_gc()) {
         // TODO(7748): Implement

src/wasm/local-decl-encoder.cc

@@ -37,8 +37,8 @@ size_t LocalDeclEncoder::Emit(byte* buffer) const {
       *pos = locals_type.depth();
       ++pos;
     }
-    if (locals_type.has_index()) {
-      LEBHelper::write_u32v(&pos, locals_type.ref_index());
+    if (locals_type.encoding_needs_heap_type()) {
+      LEBHelper::write_u32v(&pos, locals_type.heap_type_code());
     }
   }
   DCHECK_EQ(Size(), pos - buffer);
@@ -65,8 +65,9 @@ size_t LocalDeclEncoder::Size() const {
     size += LEBHelper::sizeof_u32v(p.first) +  // number of locals
             1 +                                // Opcode
             (p.second.has_depth() ? 1 : 0) +   // Inheritance depth
-            (p.second.has_index() ? LEBHelper::sizeof_u32v(p.second.ref_index())
-                                  : 0);  // ref. index
+            (p.second.encoding_needs_heap_type()
+                 ? LEBHelper::sizeof_u32v(p.second.heap_type_code())
+                 : 0);  // ref. index
   }
   return size;
 }

src/wasm/value-type.h

@@ -79,7 +79,7 @@ class ValueType {
   }
 
   constexpr bool has_index() const {
-    return is_reference_type() && heap_type() < kV8MaxWasmTypes;
+    return is_reference_type() && !is_generic_heap_type(heap_type());
   }
   constexpr bool has_depth() const { return kind() == kRtt; }
 
@@ -174,21 +174,10 @@ class ValueType {
           case kHeapExn:
             return kLocalExnRef;
           default:
-            return kLocalIndexedRef;
+            return kLocalOptRef;
         }
       case kRef:
-        switch (heap_type()) {
-          case kHeapFunc:
-            return kLocalFuncStrictRef;
-          case kHeapExtern:
-            return kLocalExternStrictRef;
-          case kHeapEq:
-            return kLocalEqStrictRef;
-          case kHeapExn:
-            return kLocalExnStrictRef;
-          default:
-            return kLocalIndexedStrictRef;
-        }
+        return kLocalRef;
       case kStmt:
         return kLocalVoid;
       default:
@@ -196,6 +185,27 @@ class ValueType {
     }
   }
 
+  constexpr bool encoding_needs_heap_type() const {
+    return kind() == kRef || kind() == kRtt ||
+           (kind() == kOptRef && !is_generic_heap_type(heap_type()));
+  }
+
+  constexpr uint32_t heap_type_code() const {
+    CONSTEXPR_DCHECK(encoding_needs_heap_type());
+    switch (heap_type()) {
+      case kHeapFunc:
+        return kLocalFuncRef;
+      case kHeapExn:
+        return kLocalExnRef;
+      case kHeapExtern:
+        return kLocalExternRef;
+      case kHeapEq:
+        return kLocalEqRef;
+      default:
+        return static_cast<uint32_t>(heap_type());
+    }
+  }
+
   static ValueType For(MachineType type) {
     switch (type.representation()) {
       case MachineRepresentation::kWord8:

src/wasm/wasm-constants.h

@@ -18,30 +18,29 @@ namespace wasm {
 constexpr uint32_t kWasmMagic = 0x6d736100;
 constexpr uint32_t kWasmVersion = 0x01;
 
-// Binary encoding of local types.
+// Binary encoding of value and heap types.
 enum ValueTypeCode : uint8_t {
+  // Current wasm types
   kLocalVoid = 0x40,
   kLocalI32 = 0x7f,
   kLocalI64 = 0x7e,
   kLocalF32 = 0x7d,
   kLocalF64 = 0x7c,
+  // Simd proposal
   kLocalS128 = 0x7b,
-
-  kLocalI8 = 0x7a,   // GC proposal placeholder
-  kLocalI16 = 0x79,  // GC proposal placeholder
-  kLocalRtt = 0x78,  // GC proposal placeholder
-
+  // reftypes, typed-funcref, and GC proposals
+  kLocalI8 = 0x7a,
+  kLocalI16 = 0x79,
   kLocalFuncRef = 0x70,
   kLocalExternRef = 0x6f,
-  kLocalEqRef = 0x69,  // GC proposal placeholder
+  // kLocalAny = 0x6e, // TODO(7748): Implement
+  kLocalEqRef = 0x6d,
+  kLocalOptRef = 0x6c,
+  kLocalRef = 0x6b,
+  // kLocalI31 = 0x6a, // TODO(7748): Implement
+  kLocalRtt = 0x69,
+  // Exception handling proposal
   kLocalExnRef = 0x68,
-  kLocalIndexedRef = 0x67,  // GC proposal placeholder
-
-  kLocalFuncStrictRef = 0x66,     // GC proposal placeholder
-  kLocalExternStrictRef = 0x65,   // GC proposal placeholder
-  kLocalEqStrictRef = 0x64,       // GC proposal placeholder
-  kLocalExnStrictRef = 0x63,      // GC proposal placeholder
-  kLocalIndexedStrictRef = 0x62,  // GC proposal placeholder
 };
 // Binary encoding of other types.
 constexpr uint8_t kWasmFunctionTypeCode = 0x60;

src/wasm/wasm-module-builder.cc

@@ -417,8 +417,8 @@ void WriteValueType(ZoneBuffer* buffer, const ValueType& type) {
   if (type.has_depth()) {
     buffer->write_u8(type.depth());
   }
-  if (type.has_index()) {
-    buffer->write_u32v(type.ref_index());
+  if (type.encoding_needs_heap_type()) {
+    buffer->write_u32v(type.heap_type_code());
   }
 }
 

test/cctest/wasm/test-run-wasm-module.cc

@@ -961,22 +961,22 @@ TEST(GcStructIdsPass) {
         U32V_1(3),            // field count
         kLocalI32,            // field 0
         U32V_1(1),            // mutability
-        kLocalIndexedRef,     // field 1
+        kLocalOptRef,         // field 1
         U32V_1(0),            // --
         U32V_1(1),            // mutability
-        kLocalIndexedRef,     // field 2
+        kLocalOptRef,         // field 2
         U32V_1(1),            // --
         U32V_1(1),            // mutability
         kWasmStructTypeCode,  // index 1 = struct(type(0), type(2))
         U32V_1(2),            // field count
-        kLocalIndexedRef,     // field 0
+        kLocalOptRef,         // field 0
         U32V_1(0),            // --
         U32V_1(1),            // mutability
-        kLocalIndexedRef,     // field 1
+        kLocalOptRef,         // field 1
         U32V_1(2),            // --
         U32V_1(1),            // mutability
         kWasmArrayTypeCode,   // index 2 = array(type(0))
-        kLocalIndexedRef,     // element type
+        kLocalOptRef,         // element type
         U32V_1(0),            // --
         U32V_1(1)             // mutability
     };
@@ -1008,7 +1008,7 @@ TEST(GcTypeIdsUndefinedIndex) {
         U32V_1(1),            // type count
         kWasmStructTypeCode,  // index 0 = struct(type(1))
         U32V_1(1),            // field count
-        kLocalIndexedRef,     // field 0
+        kLocalOptRef,         // field 0
         U32V_1(1),            // --
         U32V_1(1)             // mutability
     };
@@ -1041,7 +1041,7 @@ TEST(GcTypeIdsIllegalIndex) {
         U32V_1(2),              // type count
         kWasmStructTypeCode,    // index 0 = struct(type(1))
         U32V_1(1),              // field count
-        kLocalIndexedRef,       // field 0
+        kLocalOptRef,           // field 0
         U32V_1(1),              // --
         U32V_1(1),              // mutability
         kWasmFunctionTypeCode,  // index 1 = int32 -> int32
@@ -1080,7 +1080,7 @@ TEST(GcTypeIdsFunSigIllegalIndex) {
         U32V_1(1),              // param count
         kLocalI32,              // param 0
         U32V_1(1),              // returns count
-        kLocalIndexedRef,       // return 0
+        kLocalOptRef,           // return 0
         U32V_1(0)               // --
     };
     CompileAndInstantiateForTesting(

test/common/wasm/wasm-macro-gen.h

@@ -440,7 +440,7 @@ inline WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) {
       static_cast<byte>(fieldidx)
 #define WASM_REF_NULL(type) kExprRefNull, static_cast<byte>(type)
 #define WASM_REF_NULL_GC(type) \
-  kExprRefNull, kLocalIndexedRef, static_cast<byte>(type)
+  kExprRefNull, kLocalOptRef, static_cast<byte>(type)
 #define WASM_REF_FUNC(val) kExprRefFunc, val
 #define WASM_REF_IS_NULL(type, val) val, kExprRefIsNull, static_cast<byte>(type)
 #define WASM_REF_AS_NON_NULL(val) val, kExprRefAsNonNull
@@ -470,7 +470,7 @@ inline WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) {
 #define WASM_RETURN_CALL_INDIRECT(sig_index, ...) \
   __VA_ARGS__, kExprReturnCallIndirect, static_cast<byte>(sig_index), TABLE_ZERO
 
-#define WASM_REF_TYPE(typeidx) kLocalIndexedRef, U32V_1(typeidx)
+#define WASM_REF_TYPE(typeidx) kLocalOptRef, U32V_1(typeidx)
 
 // shift locals by 1; let (locals[0]: local_type) = value in ...
 #define WASM_LET_1_V(local_type, value, ...)                                  \

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -3636,8 +3636,8 @@ TEST_F(WasmOpcodeLengthTest, SimdExpressions) {
 }
 
 TEST_F(WasmOpcodeLengthTest, IllegalRefIndices) {
-  ExpectFailure(kExprBlock, kLocalIndexedRef, U32V_3(kV8MaxWasmTypes + 1));
-  ExpectFailure(kExprBlock, kLocalIndexedRef, U32V_4(0x01000000));
+  ExpectFailure(kExprBlock, kLocalOptRef, U32V_3(kV8MaxWasmTypes + 1));
+  ExpectFailure(kExprBlock, kLocalOptRef, U32V_4(0x01000000));
 }
 
 using TypesOfLocals = ZoneVector<ValueType>;