Optimize callers when DAE removes uninhabitable results (#7233)

In principle the optimizer should be using the fact that calls (or any
other expressions) that produce uninhabitable types will never return.
Previously, when DAE removed unused, uninhabitable results, the caller
would lose this useful information and have no way of determining that
the call would never return. Fix this by inserting an `unreachable`
after the call in the caller. Also run follow-up optimizations on the
caller because the new `unreachable` is very likely to lead to
improvements.

Author: tlively

src/passes/DeadArgumentElimination.cpp

@@ -388,7 +388,12 @@ struct DAE : public Pass {
         if (!allDropped) {
           continue;
         }
-        removeReturnValue(func.get(), calls, module);
+        if (removeReturnValue(func.get(), calls, module)) {
+          // We should optimize the callers.
+          for (auto* call : calls) {
+            worthOptimizing.insert(module->getFunction(expressionFuncs[call]));
+          }
+        }
         // TODO Removing a drop may also open optimization opportunities in the
         // callers.
         worthOptimizing.insert(func.get());
@@ -413,8 +418,17 @@ struct DAE : public Pass {
 private:
   std::unordered_map<Call*, Expression**> allDroppedCalls;
 
-  void
+  // Returns `true` if the caller should be optimized.
+  bool
   removeReturnValue(Function* func, std::vector<Call*>& calls, Module* module) {
+    // If the result type is uninhabitable, then the caller knows the call will
+    // never return. That useful information would be lost if we did nothing
+    // else when removing the return value, but we will insert an `unreachable`
+    // after the call in the caller to preserve the optimization effect. TODO:
+    // Do this for more complicated uninhabitable types such as non-nullable
+    // references to structs with non-nullable reference cycles.
+    bool wasReturnUninhabitable =
+      func->getResults().isNull() && func->getResults().isNonNullable();
     func->setResults(Type::none);
     // Remove the drops on the calls. Note that we must do this before updating
     // returns in ReturnUpdater, as there may be recursive calls of this
@@ -425,14 +439,22 @@ struct DAE : public Pass {
       auto iter = allDroppedCalls.find(call);
       assert(iter != allDroppedCalls.end());
       Expression** location = iter->second;
-      *location = call;
+      if (wasReturnUninhabitable) {
+        Builder builder(*module);
+        *location = builder.makeSequence(call, builder.makeUnreachable());
+      } else {
+        *location = call;
+      }
       // Update the call's type.
       if (call->type != Type::unreachable) {
         call->type = Type::none;
       }
     }
     // Remove any return values.
     ReturnUtils::removeReturns(func, *module);
+    // It's definitely worth optimizing the caller after inserting the
+    // unreachable.
+    return wasReturnUninhabitable;
   }
 
   // Given a function and all the calls to it, see if we can refine the type of

test/lit/passes/dae-optimizing.wast

@@ -1,7 +1,7 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
 ;; NOTE: This test was ported using port_passes_tests_to_lit.py and could be cleaned up.
 
-;; RUN: foreach %s %t wasm-opt --dae-optimizing -S -o - | filecheck %s
+;; RUN: foreach %s %t wasm-opt -all --dae-optimizing -S -o - | filecheck %s
 
 (module
  (type $0 (func (param f32) (result f32)))
@@ -14,7 +14,7 @@
  (type $2 (func (param f64 f32 f32 f64 f32 i32 i32 f64) (result i32)))
  ;; CHECK:      (global $global$0 (mut i32) (i32.const 10))
  (global $global$0 (mut i32) (i32.const 10))
- ;; CHECK:      (func $0 (result i32)
+ ;; CHECK:      (func $0 (type $3) (result i32)
  ;; CHECK-NEXT:  (local $0 i32)
  ;; CHECK-NEXT:  (local $1 i32)
  ;; CHECK-NEXT:  (drop
@@ -96,13 +96,13 @@
   )
   (i32.const -11)
  )
- ;; CHECK:      (func $1 (result f32)
+ ;; CHECK:      (func $1 (type $4) (result f32)
  ;; CHECK-NEXT:  (f32.const 0)
  ;; CHECK-NEXT: )
  (func $1 (; 1 ;) (type $0) (param $0 f32) (result f32)
   (f32.const 0)
  )
- ;; CHECK:      (func $2 (param $0 f64) (param $1 f32) (param $2 f32) (param $3 f64) (param $4 f32) (param $5 i32) (param $6 i32) (param $7 f64) (result i32)
+ ;; CHECK:      (func $2 (type $2) (param $0 f64) (param $1 f32) (param $2 f32) (param $3 f64) (param $4 f32) (param $5 i32) (param $6 i32) (param $7 f64) (result i32)
  ;; CHECK-NEXT:  (call $0)
  ;; CHECK-NEXT: )
  (func $2 (; 2 ;) (type $2) (param $0 f64) (param $1 f32) (param $2 f32) (param $3 f64) (param $4 f32) (param $5 i32) (param $6 i32) (param $7 f64) (result i32)
@@ -118,3 +118,54 @@
  )
 )
 
+;; Test that we optimize callers when there are unused uninhabitable results.
+(module
+  (func $import (import "" "") (result i32))
+  (func $impossible (import "" "") (result (ref none)))
+  ;; CHECK:      (type $0 (func (result i32)))
+
+  ;; CHECK:      (type $1 (func (result (ref none))))
+
+  ;; CHECK:      (type $2 (func))
+
+  ;; CHECK:      (import "" "" (func $import (type $0) (result i32)))
+
+  ;; CHECK:      (import "" "" (func $impossible (type $1) (result (ref none))))
+
+  ;; CHECK:      (export "export" (func $export))
+
+  ;; CHECK:      (func $export (type $0) (result i32)
+  ;; CHECK-NEXT:  (call $internal)
+  ;; CHECK-NEXT:  (unreachable)
+  ;; CHECK-NEXT: )
+  (func $export (export "export") (result i32)
+    (drop
+      ;; This should be optimized to an unreachable sequence.
+      (call $internal
+        (i32.const 0)
+      )
+    )
+    ;; Everything else should be removed by DCE.
+    (drop
+      (call $internal
+        (i32.const 1)
+      )
+    )
+    (i32.const 0)
+  )
+  ;; CHECK:      (func $internal (type $2)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (call $import)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (call $impossible)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $internal (param i32) (result (ref none))
+    ;; Prevent this from being removed entirely.
+    (drop
+      (call $import)
+    )
+    (call $impossible)
+  )
+)