Vacuum: Ignore effects at the entire function scope when possible (#5053)

Recently we added logic to ignore effects that don't "escape" past the function call.
That is, e.g. local.set only affects the current function scope, and once the call stack
is unwound it no longer matters as an effect. This moves that logic to a shared place,
and uses it in the core Vacuum logic.

The new constructor in EffectAnalyzer receives a function and then scans it as
a whole. This works just like e.g. scanning a Block as a whole (if we see a break in
the block, that has an effect only inside it, and the Block + children doesn't have a
branch effect).

Various tests are updated so they don't optimize away trivially, by adding new
return values for them.

12 files changed, 156 insertions, 43 deletions

diff --git a/src/ir/effects.h b/src/ir/effects.h
index 4b8c9a921..c2ba794d9 100644
--- a/src/ir/effects.h
+++ b/src/ir/effects.h
@@ -27,16 +27,22 @@ namespace wasm {
 
 class EffectAnalyzer {
 public:
-  EffectAnalyzer(const PassOptions& passOptions,
-                 Module& module,
-                 Expression* ast = nullptr)
+  EffectAnalyzer(const PassOptions& passOptions, Module& module)
     : ignoreImplicitTraps(passOptions.ignoreImplicitTraps),
       trapsNeverHappen(passOptions.trapsNeverHappen),
       funcEffectsMap(passOptions.funcEffectsMap), module(module),
-      features(module.features) {
-    if (ast) {
-      walk(ast);
-    }
+      features(module.features) {}
+
+  EffectAnalyzer(const PassOptions& passOptions,
+                 Module& module,
+                 Expression* ast)
+    : EffectAnalyzer(passOptions, module) {
+    walk(ast);
+  }
+
+  EffectAnalyzer(const PassOptions& passOptions, Module& module, Function* func)
+    : EffectAnalyzer(passOptions, module) {
+    walk(func);
   }
 
   bool ignoreImplicitTraps;
@@ -59,6 +65,22 @@ public:
     post();
   }
 
+  // Walk an entire function body. This will ignore effects that are not
+  // noticeable from the perspective of the caller, that is, effects that are
+  // only noticeable during the call, but "vanish" when the call stack is
+  // unwound.
+  void walk(Function* func) {
+    walk(func->body);
+
+    // We can ignore branching out of the function body - this can only be
+    // a return, and that is only noticeable in the function, not outside.
+    branchesOut = false;
+
+    // When the function exits, changes to locals cannot be noticed any more.
+    localsWritten.clear();
+    localsRead.clear();
+  }
+
   // Core effect tracking
 
   // Definitely branches out of this expression, or does a return, etc.
diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp
index 1dd91e5d7..2f816a0bd 100644
--- a/src/passes/GlobalEffects.cpp
+++ b/src/passes/GlobalEffects.cpp
@@ -49,8 +49,8 @@ struct GenerateGlobalEffects : public Pass {
         }
 
         // Gather the effects.
-        auto effects = std::make_unique<EffectAnalyzer>(
-          runner->options, *module, func->body);
+        auto effects =
+          std::make_unique<EffectAnalyzer>(runner->options, *module, func);
 
         // If the body has a call, give up - that means we can't infer a more
         // specific set of effects than the pessimistic case of just assuming
@@ -60,15 +60,6 @@ struct GenerateGlobalEffects : public Pass {
           return;
         }
 
-        // We can ignore branching out of the function body - this can only be
-        // a return, and that is only noticeable in the function, not outside.
-        effects->branchesOut = false;
-
-        // Ignore local effects - when the function exits, those become
-        // unnoticeable anyhow.
-        effects->localsWritten.clear();
-        effects->localsRead.clear();
-
         // Save the useful effects we found.
         storedEffects = std::move(effects);
       });
diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp
index 208e973da..a29c74a89 100644
--- a/src/passes/Vacuum.cpp
+++ b/src/passes/Vacuum.cpp
@@ -390,7 +390,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
       ExpressionManipulator::nop(curr->body);
     }
     if (curr->getResults() == Type::none &&
-        !EffectAnalyzer(getPassOptions(), *getModule(), curr->body)
+        !EffectAnalyzer(getPassOptions(), *getModule(), curr)
            .hasUnremovableSideEffects()) {
       ExpressionManipulator::nop(curr->body);
     }
diff --git a/test/lit/passes/vacuum-eh.wast b/test/lit/passes/vacuum-eh.wast
index de0eb0e1a..5a6a4b20b 100644
--- a/test/lit/passes/vacuum-eh.wast
+++ b/test/lit/passes/vacuum-eh.wast
@@ -22,7 +22,7 @@
     )
   )
 
-  ;; CHECK:      (func $inner-try-catch_all-test
+  ;; CHECK:      (func $inner-try-catch_all-test (result i32)
   ;; CHECK-NEXT:  (local $0 i32)
   ;; CHECK-NEXT:  (try $try0
   ;; CHECK-NEXT:   (do
@@ -31,13 +31,14 @@
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:   (catch_all
-  ;; CHECK-NEXT:    (local.set $0
+  ;; CHECK-NEXT:    (return
   ;; CHECK-NEXT:     (i32.const 1)
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $inner-try-catch_all-test (local $0 i32)
+  (func $inner-try-catch_all-test (result i32)
+    (local $0 i32)
     ;; The exception thrown in the inner try is caught by the inner catch_all,
     ;; so the outer try body does not throw and the outer try-catch can be
     ;; removed
@@ -48,7 +49,7 @@
             (throw $e (i32.const 0))
           )
           (catch_all
-            (local.set $0 (i32.const 1))
+            (return (i32.const 1))
           )
         )
       )
@@ -56,6 +57,7 @@
         (drop (pop i32))
       )
     )
+    (i32.const 2)
   )
 
   ;; CHECK:      (func $inner-try-catch-test
diff --git a/test/lit/passes/vacuum-func.wast b/test/lit/passes/vacuum-func.wast
new file mode 100644
index 000000000..fb400b7d1
--- /dev/null
+++ b/test/lit/passes/vacuum-func.wast
@@ -0,0 +1,95 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
+;; RUN: wasm-opt %s --vacuum -all -S -o - | filecheck %s
+
+;; Tests vacuuming the entire body of a function. In that case we can ignore
+;; effects like a return or changes to locals.
+
+(module
+  ;; CHECK:      (func $optimizable (param $x i32)
+  ;; CHECK-NEXT:  (local $y i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT: )
+  (func $optimizable (param $x i32)
+    (local $y i32)
+    ;; This entire function body can be optimized out. First, operations on
+    ;; locals are not observable once the function exits.
+    (local.set $x
+      (i32.const 1)
+    )
+    (local.set $y
+      (i32.const 2)
+    )
+    (drop
+      (local.get $x)
+    )
+    ;; Second, a return has no noticeable effect for the caller to notice.
+    (return)
+  )
+
+  ;; CHECK:      (func $result (param $x i32) (result i32)
+  ;; CHECK-NEXT:  (local $y i32)
+  ;; CHECK-NEXT:  (local.set $x
+  ;; CHECK-NEXT:   (i32.const 1)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.set $y
+  ;; CHECK-NEXT:   (i32.const 2)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (return
+  ;; CHECK-NEXT:   (local.get $x)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $result (param $x i32) (result i32)
+    (local $y i32)
+    ;; As above, but this function returns a value, so we cannot optimize here:
+    ;; the value must be computed and returned. (We could in theory remove just
+    ;; the parts that are valid to remove, but other passes will do so anyhow
+    ;; for the code in this test at least.)
+    (local.set $x
+      (i32.const 1)
+    )
+    (local.set $y
+      (i32.const 2)
+    )
+    (return
+      (local.get $x)
+    )
+  )
+
+  ;; CHECK:      (func $partial (param $x i32)
+  ;; CHECK-NEXT:  (local $y i32)
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $x)
+  ;; CHECK-NEXT:   (unreachable)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.set $x
+  ;; CHECK-NEXT:   (i32.const 1)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.set $y
+  ;; CHECK-NEXT:   (i32.const 2)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (return)
+  ;; CHECK-NEXT: )
+  (func $partial (param $x i32)
+    (local $y i32)
+
+    ;; As above, but with this |if| added with extra possible effects. This
+    ;; prevents optimization. (We could in theory remove just the parts that are
+    ;; valid to remove, but other passes will do so anyhow for the code in this
+    ;; test at least.)
+    (if
+      (local.get $x)
+      (unreachable)
+    )
+
+    (local.set $x
+      (i32.const 1)
+    )
+    (local.set $y
+      (i32.const 2)
+    )
+    (drop
+      (local.get $x)
+    )
+    (return)
+  )
+)
diff --git a/test/lit/passes/vacuum-intrinsics.wast b/test/lit/passes/vacuum-intrinsics.wast
index bd0f8ed60..263c07c15 100644
--- a/test/lit/passes/vacuum-intrinsics.wast
+++ b/test/lit/passes/vacuum-intrinsics.wast
@@ -11,20 +11,22 @@
   ;; CHECK:      (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects-ref (param funcref) (result (ref any))))
   (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects-ref (param funcref) (result (ref any))))
 
-  ;; CHECK:      (func $used
+  ;; CHECK:      (func $used (result i32)
   ;; CHECK-NEXT:  (local $i32 i32)
   ;; CHECK-NEXT:  (local.set $i32
   ;; CHECK-NEXT:   (call $call.without.effects
   ;; CHECK-NEXT:    (ref.func $i)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.get $i32)
   ;; CHECK-NEXT: )
-  (func $used
+  (func $used (result i32)
     (local $i32 i32)
     ;; The result is used (by the local.set), so we cannot do anything here.
     (local.set $i32
       (call $call.without.effects (ref.func $i))
     )
+    (local.get $i32)
   )
 
   ;; CHECK:      (func $unused
@@ -47,13 +49,14 @@
     )
   )
 
-  ;; CHECK:      (func $unused-fj-side-effects
+  ;; CHECK:      (func $unused-fj-side-effects (result f32)
   ;; CHECK-NEXT:  (local $f32 f32)
   ;; CHECK-NEXT:  (local.set $f32
   ;; CHECK-NEXT:   (f32.const 2.718280076980591)
   ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.get $f32)
   ;; CHECK-NEXT: )
-  (func $unused-fj-side-effects
+  (func $unused-fj-side-effects (result f32)
     (local $f32 f32)
     ;; As above, but side effects in the param. We must keep the params around
     ;; and drop them.
@@ -65,6 +68,7 @@
         (ref.func $fj)
       )
     )
+    (local.get $f32)
   )
 
   ;; CHECK:      (func $unused-unreachable
diff --git a/test/lit/passes/vacuum-tnh.wast b/test/lit/passes/vacuum-tnh.wast
index 5558735f4..37aecc5f6 100644
--- a/test/lit/passes/vacuum-tnh.wast
+++ b/test/lit/passes/vacuum-tnh.wast
@@ -139,7 +139,7 @@
   ;; NO_TNH-NEXT: )
   (func $return-nothing)
 
-  ;; YESTNH:      (func $partial (param $x (ref $struct))
+  ;; YESTNH:      (func $partial (param $x (ref $struct)) (result (ref null $struct))
   ;; YESTNH-NEXT:  (local $y (ref null $struct))
   ;; YESTNH-NEXT:  (local.set $y
   ;; YESTNH-NEXT:   (local.get $x)
@@ -147,8 +147,9 @@
   ;; YESTNH-NEXT:  (local.set $y
   ;; YESTNH-NEXT:   (local.get $x)
   ;; YESTNH-NEXT:  )
+  ;; YESTNH-NEXT:  (local.get $y)
   ;; YESTNH-NEXT: )
-  ;; NO_TNH:      (func $partial (param $x (ref $struct))
+  ;; NO_TNH:      (func $partial (param $x (ref $struct)) (result (ref null $struct))
   ;; NO_TNH-NEXT:  (local $y (ref null $struct))
   ;; NO_TNH-NEXT:  (drop
   ;; NO_TNH-NEXT:   (struct.get $struct 0
@@ -164,8 +165,9 @@
   ;; NO_TNH-NEXT:    )
   ;; NO_TNH-NEXT:   )
   ;; NO_TNH-NEXT:  )
+  ;; NO_TNH-NEXT:  (local.get $y)
   ;; NO_TNH-NEXT: )
-  (func $partial (param $x (ref $struct))
+  (func $partial (param $x (ref $struct)) (result (ref null $struct))
     (local $y (ref null $struct))
     ;; The struct.get's side effect can be ignored due to tnh, and the value is
     ;; dropped anyhow, so we can remove it. We cannot remove the local.tee
@@ -188,6 +190,7 @@
         )
       )
     )
+    (local.get $y)
   )
 
   ;; YESTNH:      (func $toplevel
diff --git a/test/passes/vacuum_all-features.txt b/test/passes/vacuum_all-features.txt
index fccb83133..f15572cac 100644
--- a/test/passes/vacuum_all-features.txt
+++ b/test/passes/vacuum_all-features.txt
@@ -4,6 +4,7 @@
  (type $1 (func (param i32)))
  (type $2 (func (result f32)))
  (type $4 (func (param i32 f64 i32 i32)))
+ (type $i32_=>_i32 (func (param i32) (result i32)))
  (type $none_=>_f64 (func (result f64)))
  (import "env" "int" (func $int (result i32)))
  (global $Int i32 (i32.const 0))
@@ -11,7 +12,7 @@
  (func $b
   (nop)
  )
- (func $l
+ (func $l (result i32)
   (local $x i32)
   (local $y i32)
   (local.set $x
@@ -23,6 +24,7 @@
   (local.set $x
    (local.get $y)
   )
+  (local.get $x)
  )
  (func $loopy (param $0 i32)
   (nop)
@@ -181,7 +183,7 @@
    )
   )
  )
- (func $if-1-block (param $x i32)
+ (func $if-1-block (param $x i32) (result i32)
   (block $out
    (if
     (local.get $x)
@@ -193,6 +195,7 @@
     )
    )
   )
+  (local.get $x)
  )
  (func $block-resize-br-gone
   (block $out
diff --git a/test/passes/vacuum_all-features.wast b/test/passes/vacuum_all-features.wast
index 1dbb6e39b..edf1e019c 100644
--- a/test/passes/vacuum_all-features.wast
+++ b/test/passes/vacuum_all-features.wast
@@ -64,7 +64,7 @@
       (nop)
     )
   )
-  (func $l (type $0)
+  (func $l (result i32)
     (local $x i32)
     (local $y i32)
     (drop
@@ -99,6 +99,7 @@
         (local.get $y)
       )
     )
+    (local.get $x)
   )
   (func $loopy (type $1) (param $0 i32)
     (loop $loop-in1
@@ -463,7 +464,7 @@
       )
     )
   )
-  (func $if-1-block (param $x i32)
+  (func $if-1-block (param $x i32) (result i32)
    (block $out
     (if
      (local.get $x)
@@ -480,6 +481,7 @@
      )
     )
    )
+   (local.get $x)
   )
   (func $block-resize-br-gone
     (block $out
diff --git a/test/wasm2js/get_local.2asm.js b/test/wasm2js/get_local.2asm.js
index d719e5f4b..da694991b 100644
--- a/test/wasm2js/get_local.2asm.js
+++ b/test/wasm2js/get_local.2asm.js
@@ -70,9 +70,6 @@ function asmFunc(importObject) {
   $3_1 = $3_1 | 0;
   $4_1 = $4_1 | 0;
   var i64toi32_i32$0 = 0, $5_1 = Math_fround(0), $6$hi = 0, $6_1 = 0, $7$hi = 0, $7_1 = 0, $8_1 = 0.0;
-  i64toi32_i32$0 = $0$hi;
-  i64toi32_i32$0 = $6$hi;
-  i64toi32_i32$0 = $7$hi;
  }
  
  function $9($0_1, $0$hi, $1_1, $2_1, $3_1, $4_1) {
diff --git a/test/wasm2js/set_local.2asm.js b/test/wasm2js/set_local.2asm.js
index df8226995..4f989942f 100644
--- a/test/wasm2js/set_local.2asm.js
+++ b/test/wasm2js/set_local.2asm.js
@@ -57,9 +57,6 @@ function asmFunc(importObject) {
   $3_1 = $3_1 | 0;
   $4_1 = $4_1 | 0;
   var i64toi32_i32$0 = 0;
-  i64toi32_i32$0 = 0;
-  i64toi32_i32$0 = 0;
-  i64toi32_i32$0 = 0;
  }
  
  function $9($0_1, $0$hi, $1_1, $2_1, $3_1, $4_1) {
diff --git a/test/wasm2js/tee_local.2asm.js b/test/wasm2js/tee_local.2asm.js
index 8bced1525..a528e7999 100644
--- a/test/wasm2js/tee_local.2asm.js
+++ b/test/wasm2js/tee_local.2asm.js
@@ -67,9 +67,6 @@ function asmFunc(importObject) {
   $3_1 = $3_1 | 0;
   $4_1 = $4_1 | 0;
   var i64toi32_i32$0 = 0;
-  i64toi32_i32$0 = 0;
-  i64toi32_i32$0 = 0;
-  i64toi32_i32$0 = 0;
  }
  
  function $9($0_1, $0$hi, $1_1, $2_1, $3_1, $4_1) {