Skip to content

Commit

Permalink
[NFC] Simplify rec group initialization in wasm-type.cpp (#5683)
Browse files Browse the repository at this point in the history
Now that we no longer support constructing basic heap types in TypeBuilder, we
can fully initialize rec groups when they are created, rather than having to
initialize them later during the build step after any basic types have been
canonicalized. Alongside that change, also simplify the process of initializing
a type builder slot to avoid completely overwriting the HeapTypeInfo in the slot
and avoid the hacky workarounds that required.
  • Loading branch information
tlively authored Apr 21, 2023
1 parent 2cd0f40 commit bd1822d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 34 deletions.
3 changes: 3 additions & 0 deletions src/wasm-type.h
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ struct Struct {

// Prevent accidental copies
Struct& operator=(const Struct&) = delete;
Struct& operator=(Struct&&) = default;
};

struct Array {
Expand All @@ -550,6 +551,8 @@ struct Array {
bool operator==(const Array& other) const { return element == other.element; }
bool operator!=(const Array& other) const { return !(*this == other); }
std::string toString() const;

Array& operator=(const Array& other) = default;
};

// TypeBuilder - allows for the construction of recursive types. Contains a
Expand Down
59 changes: 25 additions & 34 deletions src/wasm/wasm-type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ struct HeapTypeInfo {
ArrayKind,
} kind;
union {
HeapType::BasicHeapType basic;
Signature signature;
Struct struct_;
Array array;
Expand All @@ -121,7 +120,6 @@ struct HeapTypeInfo {
constexpr bool isArray() const { return kind == ArrayKind; }
constexpr bool isData() const { return isStruct() || isArray(); }

HeapTypeInfo& operator=(const HeapTypeInfo& other);
bool operator==(const HeapTypeInfo& other) const;
bool operator!=(const HeapTypeInfo& other) const { return !(*this == other); }
};
Expand Down Expand Up @@ -563,9 +561,11 @@ bool TypeInfo::operator==(const TypeInfo& other) const {
}

HeapTypeInfo::HeapTypeInfo(const HeapTypeInfo& other) {
kind = other.kind;
isTemp = other.isTemp;
supertype = other.supertype;
recGroup = other.recGroup;
recGroupIndex = other.recGroupIndex;
kind = other.kind;
switch (kind) {
case SignatureKind:
new (&signature) auto(other.signature);
Expand Down Expand Up @@ -595,14 +595,6 @@ HeapTypeInfo::~HeapTypeInfo() {
WASM_UNREACHABLE("unexpected kind");
}

HeapTypeInfo& HeapTypeInfo::operator=(const HeapTypeInfo& other) {
if (&other != this) {
this->~HeapTypeInfo();
new (this) HeapTypeInfo(other);
}
return *this;
}

bool HeapTypeInfo::operator==(const HeapTypeInfo& other) const {
return this == &other;
}
Expand Down Expand Up @@ -2247,14 +2239,21 @@ struct TypeBuilder::Impl {
// to refer to it before it is initialized. Arbitrarily choose a default
// value.
info = std::make_unique<HeapTypeInfo>(Signature());
set(Signature());
initialized = false;
info->isTemp = true;
}
void set(HeapTypeInfo&& hti) {
hti.supertype = info->supertype;
hti.recGroup = info->recGroup;
*info = std::move(hti);
info->isTemp = true;
info->kind = hti.kind;
switch (info->kind) {
case HeapTypeInfo::SignatureKind:
info->signature = hti.signature;
break;
case HeapTypeInfo::StructKind:
info->struct_ = std::move(hti.struct_);
break;
case HeapTypeInfo::ArrayKind:
info->array = hti.array;
break;
}
initialized = true;
}
HeapType get() { return HeapType(TypeID(info.get())); }
Expand Down Expand Up @@ -2326,18 +2325,22 @@ void TypeBuilder::setSubType(size_t i, HeapType super) {
sub->supertype = getHeapTypeInfo(super);
}

void TypeBuilder::createRecGroup(size_t i, size_t length) {
assert(i <= size() && i + length <= size() && "group out of bounds");
void TypeBuilder::createRecGroup(size_t index, size_t length) {
assert(index <= size() && index + length <= size() && "group out of bounds");
// Only materialize nontrivial recursion groups.
if (length < 2) {
return;
}
auto& groups = impl->recGroups;
groups.emplace_back(std::make_unique<RecGroupInfo>());
for (; length > 0; --length) {
auto& info = impl->entries[i + length - 1].info;
groups.back()->reserve(length);
for (size_t i = 0; i < length; ++i) {
auto& info = impl->entries[index + i].info;
assert(info->recGroup == nullptr && "group already assigned");
info->recGroup = groups.back().get();
auto* recGroup = groups.back().get();
recGroup->push_back(asHeapType(info));
info->recGroup = recGroup;
info->recGroupIndex = i;
}
}

Expand Down Expand Up @@ -2592,13 +2595,6 @@ validateSubtyping(const std::vector<HeapType>& types) {
std::optional<TypeBuilder::Error> canonicalizeIsorecursive(
CanonicalizationState& state,
std::vector<std::unique_ptr<RecGroupInfo>>& recGroupInfos) {
// Fill out the recursion groups.
for (auto& info : state.newInfos) {
if (info->recGroup != nullptr) {
info->recGroupIndex = info->recGroup->size();
info->recGroup->push_back(asHeapType(info));
}
}

// Map rec groups to the unique pointers to their infos.
std::unordered_map<RecGroup, std::unique_ptr<RecGroupInfo>> groupInfoMap;
Expand Down Expand Up @@ -2718,11 +2714,6 @@ TypeBuilder::BuildResult TypeBuilder::build() {
state.newInfos.emplace_back(std::move(info));
}

#if TRACE_CANONICALIZATION
std::cerr << "After replacing basic heap types:\n";
state.dump();
#endif

#if TIME_CANONICALIZATION
using instant_t = std::chrono::time_point<std::chrono::steady_clock>;
auto getMillis = [&](instant_t start, instant_t end) {
Expand Down

0 comments on commit bd1822d

Please sign in to comment.