From 81e55a5cc58498865c674ccfe0fcaff106d58ea1 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 8 Aug 2024 18:37:53 -0700 Subject: [PATCH 1/5] Add a TypeBuilder API for copying a heap type Given a function that maps the old child heap types to new child heap types, the new API takes care of copying the rest of the structure of a given heap type into a TypeBuilder slot. Use the new API in GlobalTypeRewriter::rebuildTypes. It will also be used in an upcoming type optimization. This refactoring also required adding the ability to clear the supertype of a TypeBuilder slot, which was previously not possible. --- src/ir/type-updating.cpp | 84 +++++++++++++++++----------------------- src/wasm-type.h | 68 +++++++++++++++++++++++++++++++- src/wasm/wasm-type.cpp | 4 +- 3 files changed, 104 insertions(+), 52 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index ceb37300a91..c7301974392 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -88,55 +88,47 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( // Create the temporary heap types. i = 0; + auto map = [&](HeapType type) -> HeapType { + if (auto it = typeIndices.find(type); it != typeIndices.end()) { + return typeBuilder[it->second]; + } + return type; + }; for (auto [type, _] : typeIndices) { - typeBuilder[i].setOpen(type.isOpen()); - typeBuilder[i].setShared(type.getShared()); - if (type.isSignature()) { - auto sig = type.getSignature(); - TypeList newParams, newResults; - for (auto t : sig.params) { - newParams.push_back(getTempType(t)); + typeBuilder[i].copy(type, map); + switch (type.getKind()) { + case HeapTypeKind::Func: { + auto newSig = HeapType(typeBuilder[i]).getSignature(); + modifySignature(type, newSig); + typeBuilder[i] = newSig; + break; } - for (auto t : sig.results) { - newResults.push_back(getTempType(t)); + case HeapTypeKind::Struct: { + auto newStruct = HeapType(typeBuilder[i]).getStruct(); + modifyStruct(type, newStruct); + typeBuilder[i] = newStruct; + break; } - Signature newSig(typeBuilder.getTempTupleType(newParams), - typeBuilder.getTempTupleType(newResults)); - modifySignature(type, newSig); - typeBuilder[i] = newSig; - } else if (type.isStruct()) { - auto struct_ = type.getStruct(); - // Start with a copy to get mutability/packing/etc. - auto newStruct = struct_; - for (auto& field : newStruct.fields) { - field.type = getTempType(field.type); + case HeapTypeKind::Array: { + auto newArray = HeapType(typeBuilder[i]).getArray(); + modifyArray(type, newArray); + typeBuilder[i] = newArray; + break; } - modifyStruct(type, newStruct); - typeBuilder[i] = newStruct; - } else if (type.isArray()) { - auto array = type.getArray(); - // Start with a copy to get mutability/packing/etc. - auto newArray = array; - newArray.element.type = getTempType(newArray.element.type); - modifyArray(type, newArray); - typeBuilder[i] = newArray; - } else { - WASM_UNREACHABLE("bad type"); + case HeapTypeKind::Cont: + WASM_UNREACHABLE("TODO: cont"); + case HeapTypeKind::Basic: + WASM_UNREACHABLE("unexpected kind"); } - // Apply a super, if there is one if (auto super = getDeclaredSuperType(type)) { - if (auto it = typeIndices.find(*super); it != typeIndices.end()) { - assert(it->second < i); - typeBuilder[i].subTypeOf(typeBuilder[it->second]); - } else { - typeBuilder[i].subTypeOf(*super); - } + typeBuilder[i].subTypeOf(map(*super)); + } else { + typeBuilder[i].subTypeOf(std::nullopt); } modifyTypeBuilderEntry(typeBuilder, i, type); - - i++; + ++i; } auto buildResults = typeBuilder.build(); @@ -306,18 +298,14 @@ Type GlobalTypeRewriter::getTempType(Type type) { return type; } if (type.isRef()) { - auto heapType = type.getHeapType(); - if (auto it = typeIndices.find(heapType); it != typeIndices.end()) { - return typeBuilder.getTempRefType(typeBuilder[it->second], - type.getNullability()); + auto ht = type.getHeapType(); + if (auto it = typeIndices.find(ht); it != typeIndices.end()) { + ht = typeBuilder[it->second]; } - // This type is not one that is eligible for optimizing. That is fine; just - // use it unmodified. - return type; + return typeBuilder.getTempRefType(ht, type.getNullability()); } if (type.isTuple()) { - auto& tuple = type.getTuple(); - auto newTuple = tuple; + auto newTuple = type.getTuple(); for (auto& t : newTuple) { t = getTempType(t); } diff --git a/src/wasm-type.h b/src/wasm-type.h index e2eec5f3523..c0d91ee8710 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -660,6 +660,66 @@ struct TypeBuilder { void setHeapType(size_t i, Struct&& struct_); void setHeapType(size_t i, Array array); + // Sets the heap type at index `i` to be a copy of the given heap type with + // its referenced HeapTypes to be replaced according to the provided mapping + // function. + template void copyHeapType(size_t i, HeapType type, F map) { + assert(!type.isBasic()); + if (auto super = type.getDeclaredSuperType()) { + setSubType(i, map(*super)); + } + setOpen(i, type.isOpen()); + setShared(i, type.getShared()); + + auto copySingleType = [&](Type t) -> Type { + if (t.isBasic()) { + return t; + } + assert(t.isRef()); + return getTempRefType(map(t.getHeapType()), t.getNullability()); + }; + auto copyType = [&](Type t) -> Type { + if (t.isTuple()) { + std::vector elems; + elems.reserve(t.size()); + for (auto elem : t) { + elems.push_back(copySingleType(elem)); + } + return getTempTupleType(Tuple(elems)); + } + return copySingleType(t); + }; + switch (type.getKind()) { + case HeapTypeKind::Func: { + auto sig = type.getSignature(); + setHeapType(i, Signature(copyType(sig.params), copyType(sig.results))); + return; + } + case HeapTypeKind::Struct: { + const auto& struct_ = type.getStruct(); + std::vector fields; + fields.reserve(struct_.fields.size()); + for (auto field : struct_.fields) { + field.type = copyType(field.type); + fields.push_back(field); + } + setHeapType(i, Struct(fields)); + return; + } + case HeapTypeKind::Array: { + auto elem = type.getArray().element; + elem.type = copyType(elem.type); + setHeapType(i, Array(elem)); + return; + } + case HeapTypeKind::Cont: + setHeapType(i, Continuation(map(type.getContinuation().type))); + return; + case HeapTypeKind::Basic: + WASM_UNREACHABLE("unexpected kind"); + } + } + // Gets the temporary HeapType at index `i`. This HeapType should only be used // to construct temporary Types using the methods below. HeapType getTempHeapType(size_t i); @@ -672,7 +732,7 @@ struct TypeBuilder { // Declare the HeapType being built at index `i` to be an immediate subtype of // the given HeapType. - void setSubType(size_t i, HeapType super); + void setSubType(size_t i, std::optional super); // Create a new recursion group covering slots [i, i + length). Groups must // not overlap or go out of bounds. @@ -746,7 +806,7 @@ struct TypeBuilder { builder.setHeapType(index, array); return *this; } - Entry& subTypeOf(HeapType other) { + Entry& subTypeOf(std::optional other) { builder.setSubType(index, other); return *this; } @@ -758,6 +818,10 @@ struct TypeBuilder { builder.setShared(index, share); return *this; } + template Entry& copy(HeapType type, F map) { + builder.copyHeapType(index, type, map); + return *this; + } }; Entry operator[](size_t i) { return Entry{*this, i}; } diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 173e4b8c3fe..011c51e7b7e 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -2532,10 +2532,10 @@ Type TypeBuilder::getTempRefType(HeapType type, Nullability nullable) { return markTemp(impl->typeStore.insert(TypeInfo(type, nullable))); } -void TypeBuilder::setSubType(size_t i, HeapType super) { +void TypeBuilder::setSubType(size_t i, std::optional super) { assert(i < size() && "index out of bounds"); HeapTypeInfo* sub = impl->entries[i].info.get(); - sub->supertype = getHeapTypeInfo(super); + sub->supertype = super ? getHeapTypeInfo(*super) : nullptr; } void TypeBuilder::createRecGroup(size_t index, size_t length) { From 531c4798c1e28327a17153193351c96dc77c69c7 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Mon, 12 Aug 2024 18:05:08 -0700 Subject: [PATCH 2/5] undo changes to GlobalTypeRewriter::getTempType --- src/ir/type-updating.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index c7301974392..92ec97980d6 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -298,11 +298,14 @@ Type GlobalTypeRewriter::getTempType(Type type) { return type; } if (type.isRef()) { - auto ht = type.getHeapType(); - if (auto it = typeIndices.find(ht); it != typeIndices.end()) { - ht = typeBuilder[it->second]; + auto heapType = type.getHeapType(); + if (auto it = typeIndices.find(heapType); it != typeIndices.end()) { + return typeBuilder.getTempRefType(typeBuilder[it->second], + type.getNullability()); } - return typeBuilder.getTempRefType(ht, type.getNullability()); + // This type is not one that is eligible for optimizing. That is fine; just + // use it unmodified. + return type; } if (type.isTuple()) { auto newTuple = type.getTuple(); From 392e83c3f17c5d65af98c48f3a9e2477feb7059c Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Mon, 12 Aug 2024 18:25:10 -0700 Subject: [PATCH 3/5] use copy in TypeSSA --- src/passes/TypeSSA.cpp | 15 +-------------- src/wasm-type.h | 3 +++ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/passes/TypeSSA.cpp b/src/passes/TypeSSA.cpp index 1d00efb5bff..1355b113e03 100644 --- a/src/passes/TypeSSA.cpp +++ b/src/passes/TypeSSA.cpp @@ -98,20 +98,7 @@ std::vector ensureTypesAreInNewRecGroup(RecGroup recGroup, // Make a builder and add a slot for the hash. TypeBuilder builder(num + 1); for (Index i = 0; i < num; i++) { - auto type = types[i]; - if (type.isStruct()) { - builder[i] = type.getStruct(); - } else { - // Atm this pass only needs struct and array types. If we refactor - // this function to be general purpose we'd need to extend that. TODO - assert(type.isArray()); - builder[i] = type.getArray(); - } - if (auto super = type.getDeclaredSuperType()) { - builder[i].subTypeOf(*super); - } - builder[i].setOpen(type.isOpen()); - builder[i].setShared(type.getShared()); + builder[i].copy(types[i]); } // Implement the hash as a struct with "random" fields, and add it. diff --git a/src/wasm-type.h b/src/wasm-type.h index c0d91ee8710..2cedefa2dd9 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -822,6 +822,9 @@ struct TypeBuilder { builder.copyHeapType(index, type, map); return *this; } + Entry& copy(HeapType type) { + return copy(type, [](HeapType t) { return t; }); + } }; Entry operator[](size_t i) { return Entry{*this, i}; } From 0dd13e4be6ccc40987492449bb38533001bfa049 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Mon, 12 Aug 2024 19:32:51 -0700 Subject: [PATCH 4/5] tweak to fix Windows build --- src/wasm-type.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index 2cedefa2dd9..b162c311185 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -685,7 +685,7 @@ struct TypeBuilder { for (auto elem : t) { elems.push_back(copySingleType(elem)); } - return getTempTupleType(Tuple(elems)); + return getTempTupleType(elems); } return copySingleType(t); }; From 97c1c1d9d4cd27b1a98dad6df61060edbf313949 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Mon, 12 Aug 2024 20:24:19 -0700 Subject: [PATCH 5/5] attempt to disambiguate for MSVC --- src/wasm-type.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index b162c311185..ff1358a122d 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -709,7 +709,8 @@ struct TypeBuilder { case HeapTypeKind::Array: { auto elem = type.getArray().element; elem.type = copyType(elem.type); - setHeapType(i, Array(elem)); + // MSVC gets confused without this disambiguation. + setHeapType(i, wasm::Array(elem)); return; } case HeapTypeKind::Cont: