MergeBlocks: Allow side effects in a ternary's first element (#4238)

Side effects in the first element are always ok there, as they are
not moved across anything else: they happen before their parent
both before and after the opt.

The pass just left ternary as a TODO, so do at least one part of
that now (we can do the rest as well, with some care).

This is fairly useful on array.set which has 3 operands, and the
first often has interesting things in it.

4 files changed, 138 insertions, 52 deletions

diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 9d797ae0a..b77dbef83 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -524,14 +524,10 @@ struct MergeBlocks
                        Expression*& first,
                        Expression*& second,
                        Expression*& third) {
-    // TODO: for now, just stop when we see any side effect. instead, we could
-    //       check effects carefully for reordering
     Block* outer = nullptr;
-    if (EffectAnalyzer(getPassOptions(), *getModule(), first)
-          .hasSideEffects()) {
-      return;
-    }
     outer = optimize(curr, first, outer);
+    // TODO: for now, just stop when we see any side effect after the first
+    //       item, but we could handle them carefully like we do for binaries.
     if (EffectAnalyzer(getPassOptions(), *getModule(), second)
           .hasSideEffects()) {
       return;
diff --git a/test/lit/passes/asyncify_optimize-level=1.wast b/test/lit/passes/asyncify_optimize-level=1.wast
index aa5172f26..cc1da3860 100644
--- a/test/lit/passes/asyncify_optimize-level=1.wast
+++ b/test/lit/passes/asyncify_optimize-level=1.wast
@@ -577,36 +577,36 @@
   ;; CHECK-NEXT:        (i32.const 2)
   ;; CHECK-NEXT:       )
   ;; CHECK-NEXT:      )
-  ;; CHECK-NEXT:      (select
-  ;; CHECK-NEXT:       (block (result i32)
-  ;; CHECK-NEXT:        (if
-  ;; CHECK-NEXT:         (i32.eq
-  ;; CHECK-NEXT:          (global.get $__asyncify_state)
-  ;; CHECK-NEXT:          (i32.const 2)
-  ;; CHECK-NEXT:         )
-  ;; CHECK-NEXT:         (block
-  ;; CHECK-NEXT:          (i32.store
-  ;; CHECK-NEXT:           (global.get $__asyncify_data)
-  ;; CHECK-NEXT:           (i32.sub
-  ;; CHECK-NEXT:            (i32.load
-  ;; CHECK-NEXT:             (global.get $__asyncify_data)
-  ;; CHECK-NEXT:            )
-  ;; CHECK-NEXT:            (i32.const 4)
+  ;; CHECK-NEXT:      (block (result i32)
+  ;; CHECK-NEXT:       (if
+  ;; CHECK-NEXT:        (i32.eq
+  ;; CHECK-NEXT:         (global.get $__asyncify_state)
+  ;; CHECK-NEXT:         (i32.const 2)
+  ;; CHECK-NEXT:        )
+  ;; CHECK-NEXT:        (block
+  ;; CHECK-NEXT:         (i32.store
+  ;; CHECK-NEXT:          (global.get $__asyncify_data)
+  ;; CHECK-NEXT:          (i32.sub
+  ;; CHECK-NEXT:           (i32.load
+  ;; CHECK-NEXT:            (global.get $__asyncify_data)
   ;; CHECK-NEXT:           )
+  ;; CHECK-NEXT:           (i32.const 4)
   ;; CHECK-NEXT:          )
-  ;; CHECK-NEXT:          (local.set $2
+  ;; CHECK-NEXT:         )
+  ;; CHECK-NEXT:         (local.set $2
+  ;; CHECK-NEXT:          (i32.load
   ;; CHECK-NEXT:           (i32.load
-  ;; CHECK-NEXT:            (i32.load
-  ;; CHECK-NEXT:             (global.get $__asyncify_data)
-  ;; CHECK-NEXT:            )
+  ;; CHECK-NEXT:            (global.get $__asyncify_data)
   ;; CHECK-NEXT:           )
   ;; CHECK-NEXT:          )
   ;; CHECK-NEXT:         )
   ;; CHECK-NEXT:        )
+  ;; CHECK-NEXT:       )
+  ;; CHECK-NEXT:       (select
   ;; CHECK-NEXT:        (local.get $2)
+  ;; CHECK-NEXT:        (i32.const 0)
+  ;; CHECK-NEXT:        (global.get $__asyncify_state)
   ;; CHECK-NEXT:       )
-  ;; CHECK-NEXT:       (i32.const 0)
-  ;; CHECK-NEXT:       (global.get $__asyncify_state)
   ;; CHECK-NEXT:      )
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:     (block
diff --git a/test/lit/passes/merge-blocks.wast b/test/lit/passes/merge-blocks.wast
index 89eab22ae..e6136d4da 100644
--- a/test/lit/passes/merge-blocks.wast
+++ b/test/lit/passes/merge-blocks.wast
@@ -11,6 +11,9 @@
  ;; CHECK:      (type $struct (struct (field (mut i32))))
  (type $struct (struct (field (mut i32))))
 
+ ;; CHECK:      (type $array (array (mut i32)))
+ (type $array (array (mut i32)))
+
  ;; CHECK:      (func $br_on_to_drop
  ;; CHECK-NEXT:  (nop)
  ;; CHECK-NEXT:  (drop
@@ -90,4 +93,99 @@
    (nop)
   )
  )
+
+ ;; CHECK:      (func $array.set (param $foo (ref $array))
+ ;; CHECK-NEXT:  (local $bar (ref null $array))
+ ;; CHECK-NEXT:  (nop)
+ ;; CHECK-NEXT:  (nop)
+ ;; CHECK-NEXT:  (nop)
+ ;; CHECK-NEXT:  (array.set $array
+ ;; CHECK-NEXT:   (local.tee $bar
+ ;; CHECK-NEXT:    (local.get $foo)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (i32.const 0)
+ ;; CHECK-NEXT:   (i32.const 37)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $array.set (param $foo (ref $array))
+  (local $bar (ref null $array))
+  (array.set $array
+   (block $block (result (ref null $array))
+    (nop)
+    (nop)
+    (nop)
+    ;; Side effects in the first item on the array.set do not prevent moving
+    ;; the nops outside.
+    (local.tee $bar
+     (local.get $foo)
+    )
+   )
+   (i32.const 0)
+   (i32.const 37)
+  )
+ )
+
+ ;; CHECK:      (func $array.set-no-1 (param $foo (ref $array))
+ ;; CHECK-NEXT:  (local $bar i32)
+ ;; CHECK-NEXT:  (array.set $array
+ ;; CHECK-NEXT:   (local.get $foo)
+ ;; CHECK-NEXT:   (block (result i32)
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (local.tee $bar
+ ;; CHECK-NEXT:     (i32.const 0)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (i32.const 37)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $array.set-no-1 (param $foo (ref $array))
+  (local $bar i32)
+  (array.set $array
+   (local.get $foo)
+   (block $block (result i32)
+    (nop)
+    (nop)
+    (nop)
+    ;; Side effects in the second item do prevent optimizations, currently.
+    (local.tee $bar
+     (i32.const 0)
+    )
+   )
+   (i32.const 37)
+  )
+ )
+
+ ;; CHECK:      (func $array.set-no-2 (param $foo (ref $array))
+ ;; CHECK-NEXT:  (local $bar i32)
+ ;; CHECK-NEXT:  (array.set $array
+ ;; CHECK-NEXT:   (local.get $foo)
+ ;; CHECK-NEXT:   (i32.const 0)
+ ;; CHECK-NEXT:   (block (result i32)
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (nop)
+ ;; CHECK-NEXT:    (local.tee $bar
+ ;; CHECK-NEXT:     (i32.const 37)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $array.set-no-2 (param $foo (ref $array))
+  (local $bar i32)
+  (array.set $array
+   (local.get $foo)
+   (i32.const 0)
+   (block $block (result i32)
+    (nop)
+    (nop)
+    (nop)
+    ;; Side effects in the third item do prevent optimizations, currently.
+    (local.tee $bar
+     (i32.const 37)
+    )
+   )
+  )
+ )
 )
diff --git a/test/passes/remove-unused-names_merge-blocks_all-features.txt b/test/passes/remove-unused-names_merge-blocks_all-features.txt
index 13836b3e0..9f15d1319 100644
--- a/test/passes/remove-unused-names_merge-blocks_all-features.txt
+++ b/test/passes/remove-unused-names_merge-blocks_all-features.txt
@@ -417,26 +417,28 @@
    )
   )
   (drop
-   (select
-    (block (result i32)
-     (unreachable)
-     (i32.const 20)
+   (block (result i32)
+    (unreachable)
+    (drop
+     (i32.const 30)
     )
-    (block (result i32)
-     (drop
-      (i32.const 30)
-     )
-     (i32.const 40)
+    (drop
+     (i32.const 50)
     )
-    (block (result i32)
-     (drop
-      (i32.const 50)
-     )
+    (select
+     (i32.const 20)
+     (i32.const 40)
      (i32.const 60)
     )
    )
   )
   (drop
+   (i32.const 30)
+  )
+  (drop
+   (i32.const 50)
+  )
+  (drop
    (select
     (block (result i32)
      (drop
@@ -444,18 +446,8 @@
      )
      (unreachable)
     )
-    (block (result i32)
-     (drop
-      (i32.const 30)
-     )
-     (i32.const 40)
-    )
-    (block (result i32)
-     (drop
-      (i32.const 50)
-     )
-     (i32.const 60)
-    )
+    (i32.const 40)
+    (i32.const 60)
    )
   )
   (drop