3 files changed, 85 insertions, 9 deletions

diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp
index cf7873dc0..3b0721173 100644
--- a/src/passes/param-utils.cpp
+++ b/src/passes/param-utils.cpp
@@ -62,20 +62,40 @@ bool removeParameter(const std::vector<Function*>& funcs,
   // Check if none of the calls has a param with side effects that we cannot
   // remove (as if we can remove them, we will simply do that when we remove the
   // parameter). Note: flattening the IR beforehand can help here.
-  auto hasBadEffects = [&](ExpressionList& operands) {
-    return EffectAnalyzer(runner->options, *module, operands[index])
-      .hasUnremovableSideEffects();
+  //
+  // It would also be bad if we remove a parameter that causes the type of the
+  // call to change, like this:
+  //
+  //  (call $foo
+  //    (unreachable))
+  //
+  // After removing the parameter the type should change from unreachable to
+  // something concrete. We could handle this by updating the type and then
+  // propagating that out, or by appending an unreachable after the call, but
+  // for simplicity just ignore such cases; if we are called again later then
+  // if DCE ran meanwhile then we could optimize.
+  auto hasBadEffects = [&](auto* call) {
+    auto& operands = call->operands;
+    bool hasUnremovable =
+      EffectAnalyzer(runner->options, *module, operands[index])
+        .hasUnremovableSideEffects();
+    bool wouldChangeType =
+      call->type == Type::unreachable && !call->isReturn &&
+      std::any_of(operands.begin(), operands.end(), [](Expression* operand) {
+        return operand->type == Type::unreachable;
+      });
+    return hasUnremovable || wouldChangeType;
   };
   bool callParamsAreValid =
     std::none_of(calls.begin(), calls.end(), [&](Call* call) {
-      return hasBadEffects(call->operands);
+      return hasBadEffects(call);
     });
   if (!callParamsAreValid) {
     return false;
   }
   bool callRefParamsAreValid =
     std::none_of(callRefs.begin(), callRefs.end(), [&](CallRef* call) {
-      return hasBadEffects(call->operands);
+      return hasBadEffects(call);
     });
   if (!callRefParamsAreValid) {
     return false;
diff --git a/test/lit/passes/dae_all-features.wast b/test/lit/passes/dae_all-features.wast
index edba74b80..446951561 100644
--- a/test/lit/passes/dae_all-features.wast
+++ b/test/lit/passes/dae_all-features.wast
@@ -4,14 +4,22 @@
 ;; RUN: foreach %s %t wasm-opt --dae --all-features -S -o - | filecheck %s
 
 (module
+
   ;; CHECK:      (type $none_=>_none (func))
 
   ;; CHECK:      (type $i32_=>_none (func (param i32)))
 
   ;; CHECK:      (type $none_=>_i32 (func (result i32)))
 
+  ;; CHECK:      (type $none_=>_f64 (func (result f64)))
+
   ;; CHECK:      (type $f64_=>_none (func (param f64)))
 
+  ;; CHECK:      (import "a" "b" (func $get-i32 (result i32)))
+  (import "a" "b" (func $get-i32 (result i32)))
+  ;; CHECK:      (import "a" "c" (func $get-f64 (result f64)))
+  (import "a" "c" (func $get-f64 (result f64)))
+
   ;; CHECK:      (table $0 2 2 funcref)
 
   ;; CHECK:      (elem (i32.const 0) $a9 $c8)
@@ -172,11 +180,11 @@
   )
   ;; CHECK:      (func $b6
   ;; CHECK-NEXT:  (call $a6
-  ;; CHECK-NEXT:   (unreachable)
+  ;; CHECK-NEXT:   (call $get-i32)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $b6
-    (call $a6 (unreachable) (f64.const 3.14159))
+    (call $a6 (call $get-i32) (f64.const 3.14159))
   )
   ;; CHECK:      (func $a7 (param $0 f64)
   ;; CHECK-NEXT:  (local $1 i32)
@@ -198,11 +206,11 @@
   )
   ;; CHECK:      (func $b7
   ;; CHECK-NEXT:  (call $a7
-  ;; CHECK-NEXT:   (unreachable)
+  ;; CHECK-NEXT:   (call $get-f64)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $b7
-    (call $a7 (i32.const 1) (unreachable))
+    (call $a7 (i32.const 1) (call $get-f64))
   )
   ;; CHECK:      (func $a8 (param $x i32)
   ;; CHECK-NEXT:  (nop)
diff --git a/test/lit/passes/dae_tnh.wast b/test/lit/passes/dae_tnh.wast
index fc9270cf3..2a9fb912d 100644
--- a/test/lit/passes/dae_tnh.wast
+++ b/test/lit/passes/dae_tnh.wast
@@ -32,3 +32,51 @@
     )
   )
 )
+
+(module
+  ;; CHECK:      (type $none_=>_none (func))
+
+  ;; CHECK:      (type $i32_=>_none (func (param i32)))
+
+  ;; CHECK:      (func $caller
+  ;; CHECK-NEXT:  (call $target
+  ;; CHECK-NEXT:   (unreachable)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $caller
+    ;; Removing this parameter would require the type of the call to change from
+    ;; unreachable to none. We don't handle such complexity and ignore such
+    ;; cases.
+    (call $target
+      (unreachable)
+    )
+  )
+
+  ;; CHECK:      (func $target (param $0 i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT: )
+  (func $target (param i32)
+  )
+)
+
+;; As above, but use a return_call. We can optimize that, since return_calls
+;; have type unreachable anyhow, and the optimization would not change the type.
+(module
+  ;; CHECK:      (type $none_=>_none (func))
+
+  ;; CHECK:      (func $caller
+  ;; CHECK-NEXT:  (return_call $target)
+  ;; CHECK-NEXT: )
+  (func $caller
+    (return_call $target
+      (unreachable)
+    )
+  )
+
+  ;; CHECK:      (func $target
+  ;; CHECK-NEXT:  (local $0 i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT: )
+  (func $target (param i32)
+  )
+)