diff options
author | Thomas Lively <tlively@google.com> | 2024-08-06 13:22:38 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-06 10:22:38 -0700 |
commit | bae0da03af8f4f240d659d016b6e4ee998551059 (patch) | |
tree | 02ea1286345871e9f4223bc817804e5515c625c7 /src | |
parent | 6fe3d885604bb053979f4e5adad2840ea936fd17 (diff) | |
download | binaryen-bae0da03af8f4f240d659d016b6e4ee998551059.tar.gz binaryen-bae0da03af8f4f240d659d016b6e4ee998551059.tar.bz2 binaryen-bae0da03af8f4f240d659d016b6e4ee998551059.zip |
[NFC] Add HeapType::getKind returning a new HeapTypeKind enum (#6804)
The HeapType API has functions like `isBasic()`, `isStruct()`,
`isSignature()`, etc. to test the classification of a heap type. Many
users have to call these functions in sequence and handle all or most of
the possible classifications. When we add a new kind of heap type,
finding and updating all these sites is a manual and error-prone
process.
To make adding new heap type kinds easier, introduce a new API that
returns an enum classifying the heap type. The enum can be used in
switch statements and the compiler's exhaustiveness checker will flag
use sites that need to be updated when we add a new kind of heap type.
This commit uses the new enum internally in the type system, but
follow-on commits will add new uses and convert uses of the existing
APIs to use `getKind` instead.
Diffstat (limited to 'src')
-rw-r--r-- | src/literal.h | 8 | ||||
-rw-r--r-- | src/passes/Precompute.cpp | 2 | ||||
-rw-r--r-- | src/tools/execution-results.h | 7 | ||||
-rw-r--r-- | src/tools/fuzzing/fuzzing.cpp | 3 | ||||
-rw-r--r-- | src/tools/wasm-ctor-eval.cpp | 3 | ||||
-rw-r--r-- | src/wasm-builder.h | 40 | ||||
-rw-r--r-- | src/wasm-type.h | 36 | ||||
-rw-r--r-- | src/wasm/literal.cpp | 2 | ||||
-rw-r--r-- | src/wasm/wasm-type.cpp | 177 |
9 files changed, 134 insertions, 144 deletions
diff --git a/src/literal.h b/src/literal.h index f7703d9e6..1c1128163 100644 --- a/src/literal.h +++ b/src/literal.h @@ -96,7 +96,9 @@ public: // Whether this is GC data, that is, something stored on the heap (aside from // a null or i31). This includes structs, arrays, and also strings. bool isData() const { return type.isData(); } - bool isString() const { return type.isString(); } + bool isString() const { + return type.isRef() && type.getHeapType().isMaybeShared(HeapType::string); + } bool isNull() const { return type.isNull(); } @@ -770,11 +772,11 @@ template<> struct hash<wasm::Literal> { wasm::rehash(digest, a.getFunc()); return digest; } - if (a.type.getHeapType() == wasm::HeapType::i31) { + if (a.type.getHeapType().isMaybeShared(wasm::HeapType::i31)) { wasm::rehash(digest, a.geti31(true)); return digest; } - if (a.type.isString()) { + if (a.type.getHeapType().isMaybeShared(wasm::HeapType::string)) { auto& values = a.getGCData()->values; wasm::rehash(digest, values.size()); for (auto c : values) { diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index ba04a68aa..48977dd78 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -829,7 +829,7 @@ private: return true; } // We can emit a StringConst for a string constant. - if (type.isString()) { + if (type.isRef() && type.getHeapType().isMaybeShared(HeapType::string)) { return true; } // All other reference types cannot be precomputed. Even an immutable GC diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index d9fa5cd9b..31c572e65 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -148,7 +148,8 @@ struct ExecutionResults { // simple and stable internal structures that optimizations will not alter. auto type = value.type; if (type.isRef()) { - if (type.isString() || type.getHeapType() == HeapType::i31) { + if (type.getHeapType().isMaybeShared(HeapType::string) || + type.getHeapType().isMaybeShared(HeapType::i31)) { std::cout << value << '\n'; } else if (value.isNull()) { std::cout << "null\n"; @@ -189,7 +190,9 @@ struct ExecutionResults { // TODO: Once we support optimizing under some form of open-world // assumption, we should be able to check that the types and/or structure of // GC data passed out of the module does not change. - if (a.type.isRef() && !a.type.isString()) { + if (a.type.isRef() && + !a.type.getHeapType().isMaybeShared(HeapType::string) && + !a.type.getHeapType().isMaybeShared(HeapType::i31)) { return true; } if (a != b) { diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 2aff7146e..138831860 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -2536,7 +2536,8 @@ Expression* TranslateToFuzzReader::makeConst(Type type) { if (type.isNullable() && oneIn(8)) { return builder.makeRefNull(type.getHeapType()); } - if (type.getHeapType().isString()) { + if (type.getHeapType().isMaybeShared(HeapType::string)) { + assert(!type.getHeapType().isShared() && "TODO: shared strings"); return makeStringConst(); } if (type.getHeapType().isBasic()) { diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index df4e7a9bf..11fee72fb 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -841,7 +841,8 @@ public: // externalized i31s) can be handled by the general makeConstantExpression // logic (which knows how to handle externalization, for i31s; and it also // can handle string constants). - if (!value.isData() || value.type.getHeapType().isString()) { + if (!value.isData() || + value.type.getHeapType().isMaybeShared(HeapType::string)) { return builder.makeConstantExpression(original); } diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 0ea41989d..0f9ec7b03 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -1217,26 +1217,28 @@ public: if (type.isFunction()) { return makeRefFunc(value.getFunc(), type.getHeapType()); } - if (type.isRef() && type.getHeapType().isMaybeShared(HeapType::i31)) { - return makeRefI31(makeConst(value.geti31()), - type.getHeapType().getShared()); - } - if (type.isString()) { - // The string is already WTF-16, but we need to convert from `Literals` to - // actual string. - std::stringstream wtf16; - for (auto c : value.getGCData()->values) { - auto u = c.getInteger(); - assert(u < 0x10000); - wtf16 << uint8_t(u & 0xFF); - wtf16 << uint8_t(u >> 8); + if (type.isRef()) { + if (type.getHeapType().isMaybeShared(HeapType::i31)) { + return makeRefI31(makeConst(value.geti31()), + type.getHeapType().getShared()); + } + if (type.getHeapType().isMaybeShared(HeapType::string)) { + // The string is already WTF-16, but we need to convert from `Literals` + // to actual string. + std::stringstream wtf16; + for (auto c : value.getGCData()->values) { + auto u = c.getInteger(); + assert(u < 0x10000); + wtf16 << uint8_t(u & 0xFF); + wtf16 << uint8_t(u >> 8); + } + // TODO: Use wtf16.view() once we have C++20. + return makeStringConst(wtf16.str()); + } + if (type.getHeapType().isMaybeShared(HeapType::ext)) { + return makeRefAs(ExternConvertAny, + makeConstantExpression(value.internalize())); } - // TODO: Use wtf16.view() once we have C++20. - return makeStringConst(wtf16.str()); - } - if (type.isRef() && type.getHeapType() == HeapType::ext) { - return makeRefAs(ExternConvertAny, - makeConstantExpression(value.internalize())); } TODO_SINGLE_COMPOUND(type); WASM_UNREACHABLE("unsupported constant expression"); diff --git a/src/wasm-type.h b/src/wasm-type.h index 49179b1a6..58655edc7 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -170,7 +170,6 @@ public: bool isSignature() const; bool isStruct() const; bool isArray() const; - bool isString() const; bool isDefaultable() const; Nullability getNullability() const; @@ -311,6 +310,14 @@ public: enum Shareability { Shared, Unshared }; +enum class HeapTypeKind { + Basic, + Func, + Struct, + Array, + Cont, +}; + class HeapType { // Unlike `Type`, which represents the types of values on the WebAssembly // stack, `HeapType` is used to describe the structures that reference types @@ -365,18 +372,21 @@ public: HeapType(Struct&& struct_); HeapType(Array array); + HeapTypeKind getKind() const; + constexpr bool isBasic() const { return id <= _last_basic_type; } - bool isFunction() const; - bool isData() const; - bool isSignature() const; - // Indicates whether the given type was defined to be of the form - // `(cont $ft)`. Returns false for `cont`, the top type of the continuation - // type hierarchy (and all other types). In other words, this is analogous to - // `isSignature`, but for continuation types. - bool isContinuation() const; - bool isStruct() const; - bool isArray() const; - bool isString() const; + bool isFunction() const { + return isMaybeShared(func) || getKind() == HeapTypeKind::Func; + } + bool isData() const { + auto kind = getKind(); + return isMaybeShared(string) || kind == HeapTypeKind::Struct || + kind == HeapTypeKind::Array; + } + bool isSignature() const { return getKind() == HeapTypeKind::Func; } + bool isContinuation() const { return getKind() == HeapTypeKind::Cont; } + bool isStruct() const { return getKind() == HeapTypeKind::Struct; } + bool isArray() const { return getKind() == HeapTypeKind::Array; } bool isBottom() const; bool isOpen() const; bool isShared() const { return getShared() == Shared; } @@ -385,7 +395,7 @@ public: // Check if the type is a given basic heap type, while ignoring whether it is // shared or not. - bool isMaybeShared(BasicHeapType type) { + bool isMaybeShared(BasicHeapType type) const { return isBasic() && getBasic(Unshared) == type; } diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index f2100ea71..3dcf9e350 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -439,7 +439,7 @@ bool Literal::operator==(const Literal& other) const { assert(func.is() && other.func.is()); return func == other.func; } - if (type.isString()) { + if (type.getHeapType().isMaybeShared(HeapType::string)) { return gcData->values == other.gcData->values; } if (type.isData()) { diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 339eeb0b9..aa3847a8b 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -93,12 +93,7 @@ struct HeapTypeInfo { // (i.e. contains only this type). RecGroupInfo* recGroup = nullptr; size_t recGroupIndex = 0; - enum Kind { - SignatureKind, - ContinuationKind, - StructKind, - ArrayKind, - } kind; + HeapTypeKind kind; union { Signature signature; Continuation continuation; @@ -106,19 +101,20 @@ struct HeapTypeInfo { Array array; }; - HeapTypeInfo(Signature sig) : kind(SignatureKind), signature(sig) {} + HeapTypeInfo(Signature sig) : kind(HeapTypeKind::Func), signature(sig) {} HeapTypeInfo(Continuation continuation) - : kind(ContinuationKind), continuation(continuation) {} - HeapTypeInfo(const Struct& struct_) : kind(StructKind), struct_(struct_) {} + : kind(HeapTypeKind::Cont), continuation(continuation) {} + HeapTypeInfo(const Struct& struct_) + : kind(HeapTypeKind::Struct), struct_(struct_) {} HeapTypeInfo(Struct&& struct_) - : kind(StructKind), struct_(std::move(struct_)) {} - HeapTypeInfo(Array array) : kind(ArrayKind), array(array) {} + : kind(HeapTypeKind::Struct), struct_(std::move(struct_)) {} + HeapTypeInfo(Array array) : kind(HeapTypeKind::Array), array(array) {} ~HeapTypeInfo(); - constexpr bool isSignature() const { return kind == SignatureKind; } - constexpr bool isContinuation() const { return kind == ContinuationKind; } - constexpr bool isStruct() const { return kind == StructKind; } - constexpr bool isArray() const { return kind == ArrayKind; } + constexpr bool isSignature() const { return kind == HeapTypeKind::Func; } + constexpr bool isContinuation() const { return kind == HeapTypeKind::Cont; } + constexpr bool isStruct() const { return kind == HeapTypeKind::Struct; } + constexpr bool isArray() const { return kind == HeapTypeKind::Array; } constexpr bool isData() const { return isStruct() || isArray(); } }; @@ -440,14 +436,16 @@ HeapType::BasicHeapType getBasicHeapSupertype(HeapType type) { } auto* info = getHeapTypeInfo(type); switch (info->kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: return HeapTypes::func.getBasic(info->share); - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: return HeapTypes::cont.getBasic(info->share); - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: return HeapTypes::struct_.getBasic(info->share); - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: return HeapTypes::array.getBasic(info->share); + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unexpected kind"); }; @@ -572,18 +570,20 @@ bool TypeInfo::operator==(const TypeInfo& other) const { HeapTypeInfo::~HeapTypeInfo() { switch (kind) { - case SignatureKind: + case HeapTypeKind::Func: signature.~Signature(); return; - case ContinuationKind: + case HeapTypeKind::Cont: continuation.~Continuation(); return; - case StructKind: + case HeapTypeKind::Struct: struct_.~Struct(); return; - case ArrayKind: + case HeapTypeKind::Array: array.~Array(); return; + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unexpected kind"); } @@ -814,8 +814,6 @@ bool Type::isStruct() const { return isRef() && getHeapType().isStruct(); } bool Type::isArray() const { return isRef() && getHeapType().isArray(); } -bool Type::isString() const { return isRef() && getHeapType().isString(); } - bool Type::isDefaultable() const { // A variable can get a default value if its type is concrete (unreachable // and none have no values, hence no default), and if it's a reference, it @@ -1096,56 +1094,13 @@ HeapType::HeapType(Array array) { HeapType(globalRecGroupStore.insert(std::make_unique<HeapTypeInfo>(array))); } -bool HeapType::isFunction() const { - if (isBasic()) { - return id == func; - } else { - return getHeapTypeInfo(*this)->isSignature(); - } -} - -bool HeapType::isData() const { - if (isBasic()) { - return id == struct_ || id == array || id == string; - } else { - return getHeapTypeInfo(*this)->isData(); - } -} - -bool HeapType::isSignature() const { - if (isBasic()) { - return false; - } else { - return getHeapTypeInfo(*this)->isSignature(); - } -} - -bool HeapType::isContinuation() const { +HeapTypeKind HeapType::getKind() const { if (isBasic()) { - return false; - } else { - return getHeapTypeInfo(*this)->isContinuation(); + return HeapTypeKind::Basic; } + return getHeapTypeInfo(*this)->kind; } -bool HeapType::isStruct() const { - if (isBasic()) { - return false; - } else { - return getHeapTypeInfo(*this)->isStruct(); - } -} - -bool HeapType::isArray() const { - if (isBasic()) { - return false; - } else { - return getHeapTypeInfo(*this)->isArray(); - } -} - -bool HeapType::isString() const { return *this == HeapType::string; } - bool HeapType::isBottom() const { if (isBasic()) { switch (getBasic(Unshared)) { @@ -1252,14 +1207,16 @@ std::optional<HeapType> HeapType::getSuperType() const { auto* info = getHeapTypeInfo(*this); switch (info->kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: return HeapType(func).getBasic(share); - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: return HeapType(cont).getBasic(share); - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: return HeapType(struct_).getBasic(share); - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: return HeapType(array).getBasic(share); + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unexpected kind"); } @@ -1345,13 +1302,15 @@ HeapType::BasicHeapType HeapType::getUnsharedBottom() const { } auto* info = getHeapTypeInfo(*this); switch (info->kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: return nofunc; - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: return nocont; - case HeapTypeInfo::StructKind: - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Struct: + case HeapTypeKind::Array: return none; + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unexpected kind"); } @@ -2181,18 +2140,20 @@ size_t RecGroupHasher::hash(const HeapTypeInfo& info) const { wasm::rehash(digest, info.share); wasm::rehash(digest, info.kind); switch (info.kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: hash_combine(digest, hash(info.signature)); return digest; - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: hash_combine(digest, hash(info.continuation)); return digest; - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: hash_combine(digest, hash(info.struct_)); return digest; - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: hash_combine(digest, hash(info.array)); return digest; + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unexpected kind"); } @@ -2322,14 +2283,16 @@ bool RecGroupEquator::eq(const HeapTypeInfo& a, const HeapTypeInfo& b) const { return false; } switch (a.kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: return eq(a.signature, b.signature); - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: return eq(a.continuation, b.continuation); - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: return eq(a.struct_, b.struct_); - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: return eq(a.array, b.array); + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unexpected kind"); } @@ -2436,23 +2399,25 @@ void TypeGraphWalkerBase<Self>::scanHeapType(HeapType* ht) { } auto* info = getHeapTypeInfo(*ht); switch (info->kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: taskList.push_back(Task::scan(&info->signature.results)); taskList.push_back(Task::scan(&info->signature.params)); break; - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: taskList.push_back(Task::scan(&info->continuation.type)); break; - case HeapTypeInfo::StructKind: { + case HeapTypeKind::Struct: { auto& fields = info->struct_.fields; for (auto field = fields.rbegin(); field != fields.rend(); ++field) { taskList.push_back(Task::scan(&field->type)); } break; } - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: taskList.push_back(Task::scan(&info->array.element.type)); break; + case HeapTypeKind::Basic: + WASM_UNREACHABLE("unexpected kind"); } } @@ -2480,18 +2445,20 @@ struct TypeBuilder::Impl { void set(HeapTypeInfo&& hti) { info->kind = hti.kind; switch (info->kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: info->signature = hti.signature; break; - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: info->continuation = hti.continuation; break; - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: info->struct_ = std::move(hti.struct_); break; - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: info->array = hti.array; break; + case HeapTypeKind::Basic: + WASM_UNREACHABLE("unexpected kind"); } initialized = true; } @@ -2612,14 +2579,16 @@ bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) { } SubTyper typer; switch (sub.kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: return typer.isSubType(sub.signature, super.signature); - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: return typer.isSubType(sub.continuation, super.continuation); - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: return typer.isSubType(sub.struct_, super.struct_); - case HeapTypeInfo::ArrayKind: + case HeapTypeKind::Array: return typer.isSubType(sub.array, super.array); + case HeapTypeKind::Basic: + break; } WASM_UNREACHABLE("unknown kind"); } @@ -2644,28 +2613,30 @@ validateType(HeapTypeInfo& info, std::unordered_set<HeapType>& seenTypes) { } if (info.share == Shared) { switch (info.kind) { - case HeapTypeInfo::SignatureKind: + case HeapTypeKind::Func: // TODO: Figure out and enforce shared function rules. break; - case HeapTypeInfo::ContinuationKind: + case HeapTypeKind::Cont: if (!info.continuation.type.isShared()) { return TypeBuilder::ErrorReason::InvalidFuncType; } break; - case HeapTypeInfo::StructKind: + case HeapTypeKind::Struct: for (auto& field : info.struct_.fields) { if (field.type.isRef() && !field.type.getHeapType().isShared()) { return TypeBuilder::ErrorReason::InvalidUnsharedField; } } break; - case HeapTypeInfo::ArrayKind: { + case HeapTypeKind::Array: { auto elem = info.array.element.type; if (elem.isRef() && !elem.getHeapType().isShared()) { return TypeBuilder::ErrorReason::InvalidUnsharedField; } break; } + case HeapTypeKind::Basic: + WASM_UNREACHABLE("unexpected kind"); } } return std::nullopt; |