Do not recollect types in ReorderTypes (#8195)

ReorderTypes previously collected the module types and their counts,
even though it already extends GlobalTypeRewriter, which also collects
the module types in its constructor. Not only did this duplicate work,
it was also a subtle source of bugs because ReorderTypes and
GlobalTypeRewriter collected types slightly differently. ReorderTypes
collected the binary types and GlobalTypeRewriter collected the used IR
types. This could result in different types being collected for the same
multivalue control flow signatures, causing an assertion failure when
a type was seemingly missing from the collected info.

Fix the problem and make the pass more efficient by simply not
collecting the types two separate times.

Author: tlively

src/passes/ReorderTypes.cpp

@@ -31,10 +31,6 @@ namespace wasm {
 namespace {
 
 struct ReorderingTypeRewriter : GlobalTypeRewriter {
-  using InfoMap = InsertOrderedMap<HeapType, ModuleUtils::HeapTypeInfo>;
-
-  InfoMap& typeInfo;
-
   // Use a simpler cost calculation so the effects can be seen with smaller test
   // cases.
   bool forTesting;
@@ -45,8 +41,8 @@ struct ReorderingTypeRewriter : GlobalTypeRewriter {
   static constexpr float maxFactor = 1.0;
   static constexpr Index numFactors = 21;
 
-  ReorderingTypeRewriter(Module& wasm, InfoMap& typeInfo, bool forTesting)
-    : GlobalTypeRewriter(wasm), typeInfo(typeInfo), forTesting(forTesting) {}
+  ReorderingTypeRewriter(Module& wasm, bool forTesting)
+    : GlobalTypeRewriter(wasm), forTesting(forTesting) {}
 
   std::vector<HeapType> getSortedTypes(PredecessorGraph preds) override {
     auto numTypes = preds.size();
@@ -150,11 +146,7 @@ struct ReorderTypes : Pass {
       Fatal() << "ReorderTypes requires --closed-world";
     }
 
-    // Collect the use counts for each type.
-    auto typeInfo = ModuleUtils::collectHeapTypeInfo(
-      *module, ModuleUtils::TypeInclusion::BinaryTypes);
-
-    ReorderingTypeRewriter(*module, typeInfo, forTesting).update();
+    ReorderingTypeRewriter(*module, forTesting).update();
   }
 };
 

test/lit/passes/reorder-types.wast

@@ -391,3 +391,28 @@
     (local (ref $Y))
   )
 )
+
+(module
+  ;; Regression test. ReorderTypes used to collect the types used in the binary
+  ;; for their counts, which in this case includes $multi because it is part of
+  ;; the rec group. However, GlobalTypeRewriter separately collected only the
+  ;; used IR types, which includes a standalone function type instead of $multi.
+  ;; The sort then tried to lookup the count for the standalone function type
+  ;; and crashed when it couldn't find it.
+  (rec
+    (type $multi (func (result (ref $A) (ref $B))))
+    ;; CHECK:      (rec
+    ;; CHECK-NEXT:  (type $A (sub (struct)))
+    (type $A (sub (struct)))
+    ;; CHECK:       (type $B (sub $A (struct)))
+    (type $B (sub $A (struct)))
+  )
+  ;; CHECK:      (func $test (type $3) (param $0 i32) (result (ref $A) (ref $B))
+  ;; CHECK-NEXT:  (unreachable)
+  ;; CHECK-NEXT: )
+  (func $test (param i32) (result (ref $A) (ref $B))
+    (block (type $multi) (result (ref $A) (ref $B))
+      (unreachable)
+    )
+  )
+)