Prevent pops from sinking in SimplifyLocals (#2885)

This prevents `exnref.pop`s from being sinked and separated from
`catch`. For example,
```wast
(try
  (do)
  (catch
    (local.set $0 (exnref.pop))
    (call $foo
      (i32.const 3)
      (local.get $0)
    )
  )
)
```
Here, if we sink `exnref.pop` to remove `local.set $0` and
`local.get $0`, it becomes this:
```wast
(try
  (do)
  (catch
    (nop)
    (call $foo
      (i32.const 3)
      (exnref.pop)
    )
  )
)
```
This move was possible because `i32.const 3` does not have any side
effects. But this is incorrect because now `exnref.pop` does not follow
right after `catch`.

To prevent this, this patch checks this case in `canSink` in
SimplifyLocals. When we encountered a similar case in CodeFolding, we
prevented every expression that contains `Pop` anywhere in it from being
moved, which was too conservative. This adds `danglingPop` property in
`EffectAnalyzer`, so that only pops that are not enclosed within a
`catch` count as 'dangling pops` and we only prevent those pops from
being moved or sinked.

4 files changed, 93 insertions, 1 deletions

diff --git a/test/binaryen.js/sideffects.js b/test/binaryen.js/sideffects.js
index 76bc1db68..bfb5404c2 100644
--- a/test/binaryen.js/sideffects.js
+++ b/test/binaryen.js/sideffects.js
@@ -10,6 +10,7 @@ console.log("SideEffects.WritesMemory=" + binaryen.SideEffects.WritesMemory);
 console.log("SideEffects.ImplicitTrap=" + binaryen.SideEffects.ImplicitTrap);
 console.log("SideEffects.IsAtomic=" + binaryen.SideEffects.IsAtomic);
 console.log("SideEffects.Throws=" + binaryen.SideEffects.Throws);
+console.log("SideEffects.DanglingPop=" + binaryen.SideEffects.DanglingPop);
 console.log("SideEffects.Any=" + binaryen.SideEffects.Any);
 
 var module = new binaryen.Module();
@@ -104,3 +105,12 @@ assert(
   ==
   binaryen.SideEffects.Calls | binaryen.SideEffects.Throws
 );
+
+assert(
+  binaryen.getSideEffects(
+    module.drop(module.exnref.pop()),
+    module.getFeatures()
+  )
+  ==
+  binaryen.SideEffects.DanglingPop
+);
diff --git a/test/binaryen.js/sideffects.js.txt b/test/binaryen.js/sideffects.js.txt
index 4aca0ac46..53ee474e5 100644
--- a/test/binaryen.js/sideffects.js.txt
+++ b/test/binaryen.js/sideffects.js.txt
@@ -10,4 +10,5 @@ SideEffects.WritesMemory=128
 SideEffects.ImplicitTrap=256
 SideEffects.IsAtomic=512
 SideEffects.Throws=1024
-SideEffects.Any=2047
+SideEffects.DanglingPop=2048
+SideEffects.Any=4095
diff --git a/test/passes/simplify-locals_all-features.txt b/test/passes/simplify-locals_all-features.txt
index 4a003360d..0418e6bd1 100644
--- a/test/passes/simplify-locals_all-features.txt
+++ b/test/passes/simplify-locals_all-features.txt
@@ -1894,6 +1894,7 @@
 )
 (module
  (type $none_=>_none (func))
+ (type $i32_exnref_=>_none (func (param i32 exnref)))
  (type $exnref_=>_none (func (param exnref)))
  (type $none_=>_exnref (func (result exnref)))
  (event $event$0 (attr 0) (param))
@@ -1937,4 +1938,46 @@
    )
   )
  )
+ (func $foo (param $0 i32) (param $1 exnref)
+  (nop)
+ )
+ (func $pop-cannot-be-sinked
+  (local $0 exnref)
+  (try
+   (do
+    (nop)
+   )
+   (catch
+    (local.set $0
+     (exnref.pop)
+    )
+    (call $foo
+     (i32.const 3)
+     (local.get $0)
+    )
+   )
+  )
+ )
+ (func $pop-within-catch-can-be-sinked
+  (local $0 exnref)
+  (try
+   (do
+    (nop)
+   )
+   (catch
+    (nop)
+    (call $foo
+     (i32.const 3)
+     (try (result exnref)
+      (do
+       (ref.null)
+      )
+      (catch
+       (exnref.pop)
+      )
+     )
+    )
+   )
+  )
+ )
 )
diff --git a/test/passes/simplify-locals_all-features.wast b/test/passes/simplify-locals_all-features.wast
index bfe86f448..4fdedb2f9 100644
--- a/test/passes/simplify-locals_all-features.wast
+++ b/test/passes/simplify-locals_all-features.wast
@@ -1711,4 +1711,42 @@
       )
     )
   )
+
+  (func $foo (param i32 exnref))
+  (func $pop-cannot-be-sinked (local $0 exnref)
+    (try
+      (do)
+      (catch
+        ;; This (local.set $0) of (exnref.pop) cannot be sinked to
+        ;; (local.get $0) below, because exnref.pop should follow right after
+        ;; 'catch'.
+        (local.set $0 (exnref.pop))
+        (call $foo
+          (i32.const 3)
+          (local.get $0)
+        )
+      )
+    )
+  )
+
+  (func $pop-within-catch-can-be-sinked (local $0 exnref)
+    (try
+      (do)
+      (catch
+        ;; This whole 'try' body can be sinked to eliminate local.set /
+        ;; local.get. Even though it contains a pop, it is enclosed within
+        ;; try-catch, so it is OK.
+        (local.set $0
+          (try (result exnref)
+            (do (ref.null))
+            (catch (exnref.pop))
+          )
+        )
+        (call $foo
+          (i32.const 3)
+          (local.get $0)
+        )
+      )
+    )
+  )
 )