diff options
author | Thomas Lively <7121787+tlively@users.noreply.github.com> | 2021-12-14 18:22:36 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-14 18:22:36 -0800 |
commit | 5a9d816e514add80a31410a7a180dee748ddcbb3 (patch) | |
tree | dec00267e0d48e9be959d4854cc2d8288f079fed | |
parent | 01e6429a13abd2d8f5b3d4019a273e992fdd3a66 (diff) | |
download | binaryen-5a9d816e514add80a31410a7a180dee748ddcbb3.tar.gz binaryen-5a9d816e514add80a31410a7a180dee748ddcbb3.tar.bz2 binaryen-5a9d816e514add80a31410a7a180dee748ddcbb3.zip |
[NFC] Reuse `globallyCanonicalize` for nominal types (#4395)
Hashing and comparison of nominal HeapTypeInfos previously observed their child
Types, so the Types had to be canonicalized before the HeapTypes. Unfortunately
equirecursive canonicalization requires that the HeapTypes be canonicalized
before the Types, so this was a point of divergence between the two systems.
However, #4394 updated hashing and comparison of nominal types to not depend on
child Types, so now we can harmonize the two systems by having them use the same
`globallyCanonicalize` function to canonicalize their HeapTypes followed by
their Types.
-rw-r--r-- | src/wasm/wasm-type.cpp | 53 |
1 files changed, 17 insertions, 36 deletions
diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index b2f50bfbf..9f4c9c0b9 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -2894,24 +2894,6 @@ struct Locations : TypeGraphWalker<Locations> { } }; -void globallyCanonicalizeTypes(Locations& locations) { - // Canonicalize non-tuple Types (which never directly refer to other Types) - // before tuple Types to avoid canonicalizing a tuple that still contains - // non-canonical Types. - auto canonicalizeTypes = [&](bool tuples) { - for (auto& [original, uses] : locations.types) { - if (original.isTuple() == tuples) { - Type canonical = globalTypeStore.insert(*getTypeInfo(original)); - for (Type* use : uses) { - *use = canonical; - } - } - } - }; - canonicalizeTypes(false); - canonicalizeTypes(true); -} - // Replaces temporary types and heap types in a type definition graph with their // globally canonical versions to prevent temporary types or heap type from // leaking into the global stores. @@ -2974,7 +2956,21 @@ globallyCanonicalize(std::vector<std::unique_ptr<HeapTypeInfo>>& infos) { } } - globallyCanonicalizeTypes(locations); + // Canonicalize non-tuple Types (which never directly refer to other Types) + // before tuple Types to avoid canonicalizing a tuple that still contains + // non-canonical Types. + auto canonicalizeTypes = [&](bool tuples) { + for (auto& [original, uses] : locations.types) { + if (original.isTuple() == tuples) { + Type canonical = globalTypeStore.insert(*getTypeInfo(original)); + for (Type* use : uses) { + *use = canonical; + } + } + } + }; + canonicalizeTypes(false); + canonicalizeTypes(true); #if TRACE_CANONICALIZATION std::cerr << "Final Types:\n"; @@ -3110,23 +3106,8 @@ buildNominal(std::vector<std::unique_ptr<HeapTypeInfo>> infos) { auto start = std::chrono::steady_clock::now(); #endif - // Move the HeapTypes and the Types they reach to the global stores. First - // copy reachable temporary types into the global type store and replace their - // uses in the HeapTypeInfos. Then move all the HeapTypeInfos into the global - // store. We have to copy types first because correctly hashing the - // HeapTypeInfos depends on them reaching only canonical types. That also - // means we can't reuse `globallyCanonicalize` here. - Locations locations; - for (auto& info : infos) { - HeapType root = asHeapType(info); - locations.walkRoot(&root); - } - globallyCanonicalizeTypes(locations); - - std::vector<HeapType> heapTypes; - for (auto& info : infos) { - heapTypes.push_back(globalHeapTypeStore.insert(std::move(info))); - } + // Move the HeapTypes and the Types they reach to the global stores. + std::vector<HeapType> heapTypes = globallyCanonicalize(infos); #if TIME_CANONICALIZATION auto afterMove = std::chrono::steady_clock::now(); |