Fix type merging for unknown effects (#8833)

Fixes #8831. See the incorrect optimization described in that issue.

Author: stevenfontanella

src/ir/type-updating.cpp

@@ -331,22 +331,28 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
   // effects.
   std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
     newTypeEffects;
-  for (auto& [oldType, oldEffects] : wasm.indirectCallEffects) {
-    if (!oldEffects) {
+
+  for (const auto& [oldType, newType] : oldToNewTypes) {
+    std::shared_ptr<const EffectAnalyzer>* oldEffects =
+      find_or_null(wasm.indirectCallEffects, oldType);
+    std::shared_ptr<const EffectAnalyzer>* targetEffects =
+      find_or_null(wasm.indirectCallEffects, newType);
+
+    if (!targetEffects) {
+      // Nothing to update, we already know nothing and assume all effects.
       continue;
     }
 
-    auto newType = updater.getNew(oldType);
-    std::shared_ptr<const EffectAnalyzer>& targetEffects =
-      newTypeEffects[newType];
-    if (!targetEffects) {
-      targetEffects = oldEffects;
-    } else {
-      auto merged = std::make_shared<EffectAnalyzer>(*targetEffects);
-      merged->mergeIn(*oldEffects);
-      targetEffects = merged;
+    if (!oldEffects) {
+      targetEffects->reset();
+      continue;
     }
+
+    auto merged = std::make_shared<EffectAnalyzer>(**targetEffects);
+    merged->mergeIn(**oldEffects);
+    *targetEffects = std::move(merged);
   }
+
   wasm.indirectCallEffects = std::move(newTypeEffects);
 }
 

test/lit/passes/global-effects-indirect-merge.wast

@@ -1,6 +1,6 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
-;; RUN: wasm-opt %s --all-features --closed-world --generate-global-effects --vacuum --type-merging --remove-unused-types -S -o - | filecheck %s --check-prefix VACUUM_FIRST
-;; RUN: wasm-opt %s --all-features --closed-world --generate-global-effects --type-merging --remove-unused-types --vacuum -S -o - | filecheck %s --check-prefix MERGE_FIRST
+;; RUN: foreach %s %t wasm-opt --all-features --closed-world --generate-global-effects --vacuum --type-merging --remove-unused-types -S -o - | filecheck %s --check-prefix VACUUM_FIRST
+;; RUN: foreach %s %t wasm-opt --all-features --closed-world --generate-global-effects --type-merging --remove-unused-types --vacuum -S -o - | filecheck %s --check-prefix MERGE_FIRST
 
 ;; Test that indirect call effects are preserved when types are rewritten
 ;; globally. When we rewrite $effectful and $not-effectful into the same type,
@@ -37,13 +37,7 @@
     (i32.const 0)
   )
 
-  ;; VACUUM_FIRST:      (func $f (type $1)
-  ;; VACUUM_FIRST-NEXT:  (nop)
-  ;; VACUUM_FIRST-NEXT: )
-  ;; MERGE_FIRST:      (func $f (type $1)
-  ;; MERGE_FIRST-NEXT:  (nop)
-  ;; MERGE_FIRST-NEXT: )
-  (func $f
+  (func
     ;; Reference the functions in a ref.func so that it's possible that they're
     ;; the target of indirect calls.
     (drop (ref.func $unreachable))
@@ -82,3 +76,68 @@
     )
   )
 )
+
+(module
+  (rec
+    ;; VACUUM_FIRST:       (type $nop-type (func))
+    ;; MERGE_FIRST:       (type $nop-type (func))
+    (type $nop-type (func))
+
+    (type $effectful-type (func))
+  )
+
+  ;; VACUUM_FIRST:      (type $import-type (func))
+  ;; MERGE_FIRST:      (type $import-type (func))
+  (type $import-type (func))
+
+  ;; VACUUM_FIRST:      (import "" "" (func $import (type $import-type)))
+  ;; MERGE_FIRST:      (import "" "" (func $import (type $import-type)))
+  (import "" "" (func $import (type $import-type)))
+
+  ;; VACUUM_FIRST:      (func $nop (type $nop-type)
+  ;; VACUUM_FIRST-NEXT:  (nop)
+  ;; VACUUM_FIRST-NEXT: )
+  ;; MERGE_FIRST:      (func $nop (type $nop-type)
+  ;; MERGE_FIRST-NEXT:  (nop)
+  ;; MERGE_FIRST-NEXT: )
+  (func $nop (type $nop-type)
+    (nop)
+  )
+
+  ;; VACUUM_FIRST:      (func $effectful (type $nop-type)
+  ;; VACUUM_FIRST-NEXT:  (call $import)
+  ;; VACUUM_FIRST-NEXT: )
+  ;; MERGE_FIRST:      (func $effectful (type $nop-type)
+  ;; MERGE_FIRST-NEXT:  (call $import)
+  ;; MERGE_FIRST-NEXT: )
+  (func $effectful (type $effectful-type)
+    ;; We need an extra indirection here for the test.
+    ;; If we give $import the type $effectful-type directly, the type will be
+    ;; public and --type-merging won't optimize it.
+    (call $import)
+  )
+
+  (func
+    (drop (ref.func $nop))
+    (drop (ref.func $effectful))
+  )
+
+  ;; VACUUM_FIRST:      (func $calls-effectful-type (type $0) (param $ref (ref $nop-type))
+  ;; VACUUM_FIRST-NEXT:  (call_ref $nop-type
+  ;; VACUUM_FIRST-NEXT:   (local.get $ref)
+  ;; VACUUM_FIRST-NEXT:  )
+  ;; VACUUM_FIRST-NEXT: )
+  ;; MERGE_FIRST:      (func $calls-effectful-type (type $0) (param $ref (ref $nop-type))
+  ;; MERGE_FIRST-NEXT:  (call_ref $nop-type
+  ;; MERGE_FIRST-NEXT:   (local.get $ref)
+  ;; MERGE_FIRST-NEXT:  )
+  ;; MERGE_FIRST-NEXT: )
+  (func $calls-effectful-type (param $ref (ref $effectful-type))
+    ;; See #8831. Test that effects are preserved after type merging even for
+    ;; 'unknown' effects that are represented with a missing entry in the
+    ;; `indirectCallEffects` map.
+    (call_ref $effectful-type
+      (local.get $ref)
+    )
+  )
+)