From ac6c450b38b2fe4d049eef8aaa2acd56e3be9ae2 Mon Sep 17 00:00:00 2001
From: Thomas Lively <7121787+tlively@users.noreply.github.com>
Date: Tue, 8 Feb 2022 10:55:23 -0800
Subject: Eagerly canonicalize basic types (#4507)

We were already eagerly canonicalizing basic HeapTypes when building types so
the more complicated canonicalization algorithms would not have to handle
noncanonical heap types, but we were not doing the same for Types. Equirecursive
canonicalization was properly handling noncanonical Types everywhere, but
isorecursive canonicalization was not. Rather than update isorecursive
canonicalization in multiple places, remove the special handling from
equirecursive canonicalization and canonicalize types in a single location.
---
 src/wasm/wasm-type.cpp | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

(limited to 'src')

diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
index 04b1122b6..d7ba224c8 100644
--- a/src/wasm/wasm-type.cpp
+++ b/src/wasm/wasm-type.cpp
@@ -2194,7 +2194,6 @@ std::ostream& TypePrinter::print(const Rtt& rtt) {
 }
 
 size_t FiniteShapeHasher::hash(Type type) {
-  type = asCanonical(type);
   size_t digest = wasm::hash(type.isBasic());
   if (type.isBasic()) {
     rehash(digest, type.getID());
@@ -2205,7 +2204,6 @@ size_t FiniteShapeHasher::hash(Type type) {
 }
 
 size_t FiniteShapeHasher::hash(HeapType heapType) {
-  heapType = asCanonical(heapType);
   size_t digest = wasm::hash(heapType.isBasic());
   if (heapType.isBasic()) {
     rehash(digest, heapType.getID());
@@ -2314,8 +2312,6 @@ size_t FiniteShapeHasher::hash(const Rtt& rtt) {
 }
 
 bool FiniteShapeEquator::eq(Type a, Type b) {
-  a = asCanonical(a);
-  b = asCanonical(b);
   if (a.isBasic() != b.isBasic()) {
     return false;
   } else if (a.isBasic()) {
@@ -2326,8 +2322,6 @@ bool FiniteShapeEquator::eq(Type a, Type b) {
 }
 
 bool FiniteShapeEquator::eq(HeapType a, HeapType b) {
-  a = asCanonical(a);
-  b = asCanonical(b);
   if (a.isBasic() != b.isBasic()) {
     return false;
   } else if (a.isBasic()) {
@@ -3798,7 +3792,7 @@ std::optional<TypeBuilder::Error> canonicalizeIsorecursive(
   return {};
 }
 
-void canonicalizeBasicHeapTypes(CanonicalizationState& state) {
+void canonicalizeBasicTypes(CanonicalizationState& state) {
   // Replace heap types backed by BasicKind HeapTypeInfos with their
   // corresponding BasicHeapTypes. The heap types backed by BasicKind
   // HeapTypeInfos exist only to support building basic types in a TypeBuilder
@@ -3814,6 +3808,25 @@ void canonicalizeBasicHeapTypes(CanonicalizationState& state) {
     }
   }
   state.update(replacements);
+
+  if (replacements.size()) {
+    // Canonicalizing basic heap types may cause their parent types to become
+    // canonicalizable as well, for example after creating `(ref null extern)`
+    // we can futher canonicalize to `externref`.
+    struct TypeCanonicalizer : TypeGraphWalkerBase<TypeCanonicalizer> {
+      void scanType(Type* type) {
+        if (type->isTuple()) {
+          TypeGraphWalkerBase<TypeCanonicalizer>::scanType(type);
+        } else {
+          *type = asCanonical(*type);
+        }
+      }
+    };
+    for (auto& info : state.newInfos) {
+      auto root = asHeapType(info);
+      TypeCanonicalizer{}.walkRoot(&root);
+    }
+  }
 }
 
 } // anonymous namespace
@@ -3842,7 +3855,7 @@ TypeBuilder::BuildResult TypeBuilder::build() {
 
   // Eagerly replace references to built basic heap types so the more
   // complicated canonicalization algorithms don't need to consider them.
-  canonicalizeBasicHeapTypes(state);
+  canonicalizeBasicTypes(state);
 
 #if TRACE_CANONICALIZATION
   std::cerr << "After replacing basic heap types:\n";
-- 
cgit v1.2.3