Fix a trivial CodePushing bug with looking at the wrong index (#5252)

Pretty simple logic bug, but it ended up causing us to not optimize sometimes.
Sadly the original tests happened to not have anything that depended on the
index in isolation.

Fix + add comprehensive tests for using that index properly. Also test the
call.without.effects intrinsic, which is orthoginal to this, but also worth testing
as it is a big use case here.

2 files changed, 266 insertions, 11 deletions

diff --git a/test/lit/passes/code-pushing_into_if.wast b/test/lit/passes/code-pushing_into_if.wast
index e856f446c..06e64dab6 100644
--- a/test/lit/passes/code-pushing_into_if.wast
+++ b/test/lit/passes/code-pushing_into_if.wast
@@ -1,7 +1,10 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
-;; RUN: wasm-opt %s --code-pushing -S -o - | filecheck %s
+;; RUN: wasm-opt %s --code-pushing --enable-reference-types -S -o - | filecheck %s
 
 (module
+  ;; CHECK:      (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param i32 funcref) (result i32)))
+  (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param i32 funcref) (result i32)))
+
   ;; CHECK:      (func $if-nop (param $p i32)
   ;; CHECK-NEXT:  (local $x i32)
   ;; CHECK-NEXT:  (local.set $x
@@ -648,4 +651,256 @@
       (drop (local.get $x))
     )
   )
+
+  ;; CHECK:      (func $sink-call (param $p i32) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:   (block
+  ;; CHECK-NEXT:    (local.set $temp
+  ;; CHECK-NEXT:     (call $call.without.effects
+  ;; CHECK-NEXT:      (i32.const 1234)
+  ;; CHECK-NEXT:      (ref.func $sink-call)
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (return
+  ;; CHECK-NEXT:     (local.get $temp)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (i32.const 0)
+  ;; CHECK-NEXT: )
+  (func $sink-call (param $p i32) (result i32)
+    (local $temp i32)
+
+    ;; This local has a call, but the call is an intrinsic indicating no
+    ;; effects, so it is safe to sink into the if.
+    (local.set $temp
+      (call $call.without.effects
+        (i32.const 1234)
+        (ref.func $sink-call)
+      )
+    )
+    (if
+      (local.get $p)
+      (return
+        (local.get $temp)
+      )
+    )
+    (i32.const 0)
+  )
+
+  ;; CHECK:      (func $no-sink-call (param $p i32) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (local.set $temp
+  ;; CHECK-NEXT:   (call $call.without.effects
+  ;; CHECK-NEXT:    (i32.const 1234)
+  ;; CHECK-NEXT:    (ref.func $no-sink-call)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:   (return
+  ;; CHECK-NEXT:    (local.get $temp)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.get $temp)
+  ;; CHECK-NEXT: )
+  (func $no-sink-call (param $p i32) (result i32)
+    (local $temp i32)
+
+    ;; As above, but now after the if we have a get of the temp local, so we
+    ;; cannot sink. This + the previous testcase show we scan for such local
+    ;; uses in exactly the right places.
+    (local.set $temp
+      (call $call.without.effects
+        (i32.const 1234)
+        (ref.func $no-sink-call)
+      )
+    )
+    (if
+      (local.get $p)
+      (return
+        (local.get $temp)
+      )
+    )
+    (local.get $temp) ;; this line changed.
+  )
+
+  ;; CHECK:      (func $no-sink-call-2 (param $p i32) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (local.set $temp
+  ;; CHECK-NEXT:   (call $call.without.effects
+  ;; CHECK-NEXT:    (i32.const 1234)
+  ;; CHECK-NEXT:    (ref.func $no-sink-call-2)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:   (return
+  ;; CHECK-NEXT:    (local.get $temp)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (local.get $temp)
+  ;; CHECK-NEXT: )
+  (func $no-sink-call-2 (param $p i32) (result i32)
+    (local $temp i32)
+
+    ;; As above, but add a nop before the final value. We still should not
+    ;; optimize.
+    (local.set $temp
+      (call $call.without.effects
+        (i32.const 1234)
+        (ref.func $no-sink-call-2)
+      )
+    )
+    (if
+      (local.get $p)
+      (return
+        (local.get $temp)
+      )
+    )
+    (nop) ;; this line was added.
+    (local.get $temp)
+  )
+
+  ;; CHECK:      (func $no-sink-call-3 (param $p i32) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (local.set $temp
+  ;; CHECK-NEXT:   (call $call.without.effects
+  ;; CHECK-NEXT:    (i32.const 1234)
+  ;; CHECK-NEXT:    (ref.func $no-sink-call-3)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:   (return
+  ;; CHECK-NEXT:    (local.get $temp)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (local.get $temp)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (i32.const 0)
+  ;; CHECK-NEXT: )
+  (func $no-sink-call-3 (param $p i32) (result i32)
+    (local $temp i32)
+
+    ;; As above, but add a nop after the get of the local after the if. We still
+    ;; should not optimize.
+    (local.set $temp
+      (call $call.without.effects
+        (i32.const 1234)
+        (ref.func $no-sink-call-3)
+      )
+    )
+    (if
+      (local.get $p)
+      (return
+        (local.get $temp)
+      )
+    )
+    (nop)
+    (drop
+      (local.get $temp) ;; this get is now dropped.
+    )
+    (nop) ;; this line was added;
+    (i32.const 0) ;; this line was added.
+  )
+
+  ;; CHECK:      (func $sink-call-3 (param $p i32) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:   (block
+  ;; CHECK-NEXT:    (local.set $temp
+  ;; CHECK-NEXT:     (call $call.without.effects
+  ;; CHECK-NEXT:      (i32.const 1234)
+  ;; CHECK-NEXT:      (ref.func $no-sink-call-3)
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (return
+  ;; CHECK-NEXT:     (local.get $temp)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (i32.const 0)
+  ;; CHECK-NEXT: )
+  (func $sink-call-3 (param $p i32) (result i32)
+    (local $temp i32)
+
+    ;; As above, but stop reading the relevant local after the if, keeping the
+    ;; number of other items unchanged. This verifies the presence of multiple
+    ;; items is not a problem and we can optimize.
+    (local.set $temp
+      (call $call.without.effects
+        (i32.const 1234)
+        (ref.func $no-sink-call-3)
+      )
+    )
+    (if
+      (local.get $p)
+      (return
+        (local.get $temp)
+      )
+    )
+    (nop)
+    (drop
+      (local.get $p) ;; this get now reads $p
+    )
+    (nop)
+    (i32.const 0)
+  )
+
+  ;; CHECK:      (func $no-sink-call-sub (param $p i32) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (local $other i32)
+  ;; CHECK-NEXT:  (local.set $temp
+  ;; CHECK-NEXT:   (call $call.without.effects
+  ;; CHECK-NEXT:    (local.tee $other
+  ;; CHECK-NEXT:     (i32.const 1234)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (ref.func $no-sink-call)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (local.get $p)
+  ;; CHECK-NEXT:   (return
+  ;; CHECK-NEXT:    (local.get $temp)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (i32.const 0)
+  ;; CHECK-NEXT: )
+  (func $no-sink-call-sub (param $p i32) (result i32)
+    (local $temp i32)
+    (local $other i32)
+
+    ;; The call has no effects, but one of the arguments to it does, so we
+    ;; cannot optimize.
+    (local.set $temp
+      (call $call.without.effects
+        (local.tee $other ;; an effect
+          (i32.const 1234)
+        )
+        (ref.func $no-sink-call)
+      )
+    )
+    (if
+      (local.get $p)
+      (return
+        (local.get $temp)
+      )
+    )
+    (i32.const 0)
+  )
 )
diff --git a/test/lit/passes/inlining-optimizing_optimize-level=3.wast b/test/lit/passes/inlining-optimizing_optimize-level=3.wast
index cd51a3121..feaa3171a 100644
--- a/test/lit/passes/inlining-optimizing_optimize-level=3.wast
+++ b/test/lit/passes/inlining-optimizing_optimize-level=3.wast
@@ -7917,16 +7917,6 @@
  ;; CHECK-NEXT:                              )
  ;; CHECK-NEXT:                             )
  ;; CHECK-NEXT:                             (block $do-once99
- ;; CHECK-NEXT:                              (local.set $20
- ;; CHECK-NEXT:                               (select
- ;; CHECK-NEXT:                                (local.get $11)
- ;; CHECK-NEXT:                                (i32.add
- ;; CHECK-NEXT:                                 (local.get $12)
- ;; CHECK-NEXT:                                 (i32.const 4)
- ;; CHECK-NEXT:                                )
- ;; CHECK-NEXT:                                (local.get $26)
- ;; CHECK-NEXT:                               )
- ;; CHECK-NEXT:                              )
  ;; CHECK-NEXT:                              (call $_pad
  ;; CHECK-NEXT:                               (local.get $0)
  ;; CHECK-NEXT:                               (i32.const 48)
@@ -7937,6 +7927,16 @@
  ;; CHECK-NEXT:                                  (i32.const 0)
  ;; CHECK-NEXT:                                 )
  ;; CHECK-NEXT:                                 (block (result i32)
+ ;; CHECK-NEXT:                                  (local.set $20
+ ;; CHECK-NEXT:                                   (select
+ ;; CHECK-NEXT:                                    (local.get $11)
+ ;; CHECK-NEXT:                                    (i32.add
+ ;; CHECK-NEXT:                                     (local.get $12)
+ ;; CHECK-NEXT:                                     (i32.const 4)
+ ;; CHECK-NEXT:                                    )
+ ;; CHECK-NEXT:                                    (local.get $26)
+ ;; CHECK-NEXT:                                   )
+ ;; CHECK-NEXT:                                  )
  ;; CHECK-NEXT:                                  (local.set $18
  ;; CHECK-NEXT:                                   (i32.eqz
  ;; CHECK-NEXT:                                    (local.get $18)