From 782fa8d80fbb168f990d4ff25c28f0ad488e3544 Mon Sep 17 00:00:00 2001 From: Manos Koukoutos Date: Tue, 26 May 2020 06:45:30 +0000 Subject: [PATCH] [wasm-gc] Add packed types to ValueType Motivation: In the wasm-gc proposal, structs and arrays are allowed to store elements of packed types i8 and i16. Changes: - Add i8 and i16 to ValueType. - Fix all case switches to handle the new cases. - Add a couple helper methods to ValueType and improve the implementation/usage of a couple more. Bug: v8:7748 Change-Id: I527cfe5acf5d877fc38e4212174ba9f9de5c40ad Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215046 Reviewed-by: Tobias Tebbi Reviewed-by: Jakob Kummerow Commit-Queue: Manos Koukoutos Cr-Commit-Position: refs/heads/master@{#67994} --- src/compiler/wasm-compiler.cc | 16 +++++++++++++++- src/diagnostics/objects-printer.cc | 4 ++++ src/wasm/function-body-decoder-impl.h | 10 ++++++---- src/wasm/graph-builder-interface.cc | 2 ++ src/wasm/module-instantiate.cc | 2 ++ src/wasm/value-type.h | 14 +++++++++++--- src/wasm/wasm-constants.h | 2 ++ src/wasm/wasm-interpreter.cc | 10 ++++++++++ src/wasm/wasm-js.cc | 6 ++++++ src/wasm/wasm-module-builder.cc | 2 ++ src/wasm/wasm-objects.cc | 2 ++ test/common/wasm/wasm-module-runner.cc | 2 ++ 12 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 0d74857b29..1b1d3ecd9a 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -2126,6 +2126,8 @@ Node* WasmGraphBuilder::Throw(uint32_t exception_index, STORE_FIXED_ARRAY_SLOT_ANY(values_array, index, value); ++index; break; + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: case wasm::ValueType::kStmt: case wasm::ValueType::kBottom: UNREACHABLE(); @@ -2279,6 +2281,8 @@ Node* WasmGraphBuilder::GetExceptionValues(Node* except_obj, value = LOAD_FIXED_ARRAY_SLOT_ANY(values_array, index); ++index; break; + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: case wasm::ValueType::kStmt: case wasm::ValueType::kBottom: UNREACHABLE(); @@ -5507,6 +5511,9 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { // TODO(7748): Implement properly. For now, we just expose the raw // object for testing. return node; + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: + UNIMPLEMENTED(); case wasm::ValueType::kStmt: case wasm::ValueType::kBottom: UNREACHABLE(); @@ -5616,7 +5623,14 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { DCHECK(enabled_features_.has_bigint()); return BuildChangeBigIntToInt64(input, js_context); - default: + case wasm::ValueType::kS128: + case wasm::ValueType::kRef: + case wasm::ValueType::kOptRef: + case wasm::ValueType::kEqRef: + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: + case wasm::ValueType::kBottom: + case wasm::ValueType::kStmt: UNREACHABLE(); break; } diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc index ce2da89d92..fd9e77477a 100644 --- a/src/diagnostics/objects-printer.cc +++ b/src/diagnostics/objects-printer.cc @@ -1690,6 +1690,8 @@ void WasmStruct::WasmStructPrint(std::ostream& os) { // NOLINT case wasm::ValueType::kF64: os << base::ReadUnalignedValue(field_address); break; + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: case wasm::ValueType::kS128: case wasm::ValueType::kAnyRef: case wasm::ValueType::kFuncRef: @@ -1731,6 +1733,8 @@ void WasmArray::WasmArrayPrint(std::ostream& os) { // NOLINT PrintTypedArrayElements(os, reinterpret_cast(data_ptr), len, true); break; + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: case wasm::ValueType::kS128: case wasm::ValueType::kAnyRef: case wasm::ValueType::kFuncRef: diff --git a/src/wasm/function-body-decoder-impl.h b/src/wasm/function-body-decoder-impl.h index e15f45346f..9d97d8a6db 100644 --- a/src/wasm/function-body-decoder-impl.h +++ b/src/wasm/function-body-decoder-impl.h @@ -333,9 +333,11 @@ ValueType read_value_type(Decoder* decoder, const byte* pc, "--experimental-wasm-simd"); return kWasmBottom; case kLocalVoid: - // Although void is included in ValueType, it is technically not a value - // type and is only used to indicate a void block return type. The caller - // of this function is responsible to check for its presence separately. + case kLocalI8: + case kLocalI16: + // Although these types are included in ValueType, they are technically + // not value types and are only used in specific contexts. The caller of + // this function is responsible to check for them separately. break; } // Malformed modules specifying invalid types can get here. @@ -2275,7 +2277,7 @@ class WasmFullDecoder : public WasmDecoder { Value fval = Pop(); Value tval = Pop(0, fval.type); ValueType type = tval.type == kWasmBottom ? fval.type : tval.type; - if (type.IsSubTypeOf(kWasmAnyRef)) { + if (type.IsReferenceType()) { this->error( "select without type is only valid for value type inputs"); break; diff --git a/src/wasm/graph-builder-interface.cc b/src/wasm/graph-builder-interface.cc index e7d9e42cdd..79d25412db 100644 --- a/src/wasm/graph-builder-interface.cc +++ b/src/wasm/graph-builder-interface.cc @@ -795,6 +795,8 @@ class WasmGraphBuildingInterface { TFNode* DefaultValue(ValueType type) { switch (type.kind()) { + case ValueType::kI8: + case ValueType::kI16: case ValueType::kI32: return builder_->Int32Constant(0); case ValueType::kI64: diff --git a/src/wasm/module-instantiate.cc b/src/wasm/module-instantiate.cc index 9dfc1e1608..fb8aacf20e 100644 --- a/src/wasm/module-instantiate.cc +++ b/src/wasm/module-instantiate.cc @@ -815,6 +815,8 @@ void InstanceBuilder::WriteGlobalValue(const WasmGlobal& global, case ValueType::kStmt: case ValueType::kS128: case ValueType::kBottom: + case ValueType::kI8: + case ValueType::kI16: UNREACHABLE(); } TRACE(", type = %s (from WebAssembly.Global)\n", global.type.type_name()); diff --git a/src/wasm/value-type.h b/src/wasm/value-type.h index 357dafbe2c..1c6062de45 100644 --- a/src/wasm/value-type.h +++ b/src/wasm/value-type.h @@ -49,6 +49,8 @@ class Simd128; V(F32, 2, F32, Float32, 'f', "f32") \ V(F64, 3, F64, Float64, 'd', "f64") \ V(S128, 4, S128, Simd128, 's', "s128") \ + V(I8, 1, I8, Int8, 'b', "i8") \ + V(I16, 1, I16, Int16, 'h', "i16") \ V(AnyRef, kSystemPointerSizeLog2, AnyRef, TaggedPointer, 'r', "anyref") \ V(FuncRef, kSystemPointerSizeLog2, FuncRef, TaggedPointer, 'a', "funcref") \ V(NullRef, kSystemPointerSizeLog2, NullRef, TaggedPointer, 'n', "nullref") \ @@ -129,9 +131,7 @@ class ValueType { } constexpr bool IsReferenceType() const { - return kind() == kAnyRef || kind() == kFuncRef || kind() == kNullRef || - kind() == kExnRef || kind() == kRef || kind() == kOptRef || - kind() == kEqRef; + return kAnyRef <= kind() && kind() <= kEqRef; } // TODO(7748): Extend this with struct and function subtyping. @@ -226,6 +226,12 @@ class ValueType { return kTypeName[kind()]; } + constexpr bool IsPacked() const { return kind() == kI8 || kind() == kI16; } + + constexpr ValueType Unpack() const { + return IsPacked() ? ValueType(kI32) : *this; + } + private: using KindField = base::BitField; using RefIndexField = base::BitField; @@ -255,6 +261,8 @@ constexpr ValueType kWasmExnRef = ValueType(ValueType::kExnRef); constexpr ValueType kWasmFuncRef = ValueType(ValueType::kFuncRef); constexpr ValueType kWasmNullRef = ValueType(ValueType::kNullRef); constexpr ValueType kWasmS128 = ValueType(ValueType::kS128); +constexpr ValueType kWasmI8 = ValueType(ValueType::kI8); +constexpr ValueType kWasmI16 = ValueType(ValueType::kI16); constexpr ValueType kWasmStmt = ValueType(ValueType::kStmt); constexpr ValueType kWasmBottom = ValueType(ValueType::kBottom); diff --git a/src/wasm/wasm-constants.h b/src/wasm/wasm-constants.h index b860ae692c..793254b5d2 100644 --- a/src/wasm/wasm-constants.h +++ b/src/wasm/wasm-constants.h @@ -35,6 +35,8 @@ enum ValueTypeCode : uint8_t { kLocalI31Ref = 0x6a, // GC proposal kLocalRttRef = 0x69, // GC proposal kLocalExnRef = 0x68, + kLocalI8 = 0x7a, // GC proposal + kLocalI16 = 0x79 // GC proposal }; // Binary encoding of other types. constexpr uint8_t kWasmFunctionTypeCode = 0x60; diff --git a/src/wasm/wasm-interpreter.cc b/src/wasm/wasm-interpreter.cc index 1a076e60cd..d9b9078f98 100644 --- a/src/wasm/wasm-interpreter.cc +++ b/src/wasm/wasm-interpreter.cc @@ -1444,6 +1444,8 @@ class ThreadImpl { } case ValueType::kStmt: case ValueType::kBottom: + case ValueType::kI8: + case ValueType::kI16: UNREACHABLE(); break; } @@ -2877,6 +2879,8 @@ class ThreadImpl { case ValueType::kEqRef: // TODO(7748): Implement these. UNIMPLEMENTED(); + case ValueType::kI8: + case ValueType::kI16: case ValueType::kStmt: case ValueType::kBottom: UNREACHABLE(); @@ -2987,6 +2991,8 @@ class ThreadImpl { case ValueType::kEqRef: // TODO(7748): Implement these. UNIMPLEMENTED(); + case ValueType::kI8: + case ValueType::kI16: case ValueType::kStmt: case ValueType::kBottom: UNREACHABLE(); @@ -3430,6 +3436,8 @@ class ThreadImpl { global_buffer->set(global_index, *ref); break; } + case ValueType::kI8: + case ValueType::kI16: case ValueType::kStmt: case ValueType::kBottom: UNREACHABLE(); @@ -3835,6 +3843,8 @@ class ThreadImpl { case ValueType::kEqRef: PrintF("(func|null|exn|opt|eq|)ref:unimplemented"); break; + case ValueType::kI8: + case ValueType::kI16: case ValueType::kBottom: UNREACHABLE(); break; diff --git a/src/wasm/wasm-js.cc b/src/wasm/wasm-js.cc index b8affe6422..7b5f3b4729 100644 --- a/src/wasm/wasm-js.cc +++ b/src/wasm/wasm-js.cc @@ -1369,6 +1369,8 @@ void WebAssemblyGlobal(const v8::FunctionCallbackInfo& args) { case i::wasm::ValueType::kEqRef: // TODO(7748): Implement these. UNIMPLEMENTED(); + case i::wasm::ValueType::kI8: + case i::wasm::ValueType::kI16: case i::wasm::ValueType::kStmt: case i::wasm::ValueType::kS128: case i::wasm::ValueType::kBottom: @@ -1834,6 +1836,8 @@ void WebAssemblyGlobalGetValueCommon( case i::wasm::ValueType::kEqRef: // TODO(7748): Implement these. UNIMPLEMENTED(); + case i::wasm::ValueType::kI8: + case i::wasm::ValueType::kI16: case i::wasm::ValueType::kBottom: case i::wasm::ValueType::kStmt: case i::wasm::ValueType::kS128: @@ -1924,6 +1928,8 @@ void WebAssemblyGlobalSetValue( case i::wasm::ValueType::kEqRef: // TODO(7748): Implement these. UNIMPLEMENTED(); + case i::wasm::ValueType::kI8: + case i::wasm::ValueType::kI16: case i::wasm::ValueType::kBottom: case i::wasm::ValueType::kStmt: case i::wasm::ValueType::kS128: diff --git a/src/wasm/wasm-module-builder.cc b/src/wasm/wasm-module-builder.cc index 182b0132d7..86b1727516 100644 --- a/src/wasm/wasm-module-builder.cc +++ b/src/wasm/wasm-module-builder.cc @@ -595,6 +595,8 @@ void WasmModuleBuilder::WriteTo(ZoneBuffer* buffer) const { case ValueType::kEqRef: buffer->write_u8(kExprRefNull); break; + case ValueType::kI8: + case ValueType::kI16: case ValueType::kStmt: case ValueType::kS128: case ValueType::kBottom: diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index 2883467889..68557c7cd7 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -1738,6 +1738,8 @@ uint32_t WasmExceptionPackage::GetEncodedSize( break; case wasm::ValueType::kStmt: case wasm::ValueType::kBottom: + case wasm::ValueType::kI8: + case wasm::ValueType::kI16: UNREACHABLE(); } } diff --git a/test/common/wasm/wasm-module-runner.cc b/test/common/wasm/wasm-module-runner.cc index 83e2f3824b..e6e95df5d5 100644 --- a/test/common/wasm/wasm-module-runner.cc +++ b/test/common/wasm/wasm-module-runner.cc @@ -112,6 +112,8 @@ bool InterpretWasmModuleForTesting(Isolate* isolate, arguments[i] = WasmValue(Handle::cast(isolate->factory()->null_value())); break; + case ValueType::kI8: + case ValueType::kI16: case ValueType::kStmt: case ValueType::kBottom: case ValueType::kS128: