LocalCSE: ignore traps (#4085)

If we replace

A
A
A

with

(local.set A)
(local.get)
(local.get)

then it is ok for A to trap (so long as it does so deterministically), as if
it does trap then the first appearance will do so, and the others not
be reached anyhow.

This helps GC code as often there are repeated struct.gets and such that
may trap.

2 files changed, 50 insertions, 9 deletions

diff --git a/test/lit/passes/local-cse.wast b/test/lit/passes/local-cse.wast
index 5cdf712f8..6d1b8a824 100644
--- a/test/lit/passes/local-cse.wast
+++ b/test/lit/passes/local-cse.wast
@@ -266,20 +266,21 @@
   )
 
   ;; CHECK:      (func $loads
+  ;; CHECK-NEXT:  (local $0 i32)
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (i32.load
-  ;; CHECK-NEXT:    (i32.const 10)
+  ;; CHECK-NEXT:   (local.tee $0
+  ;; CHECK-NEXT:    (i32.load
+  ;; CHECK-NEXT:     (i32.const 10)
+  ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (i32.load
-  ;; CHECK-NEXT:    (i32.const 10)
-  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (local.get $0)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $loads
-    ;; The possible trap on loads prevents optimization.
-    ;; TODO: optimize that too
+    ;; The possible trap on loads does not prevent optimization, since if we
+    ;; trap then it doesn't matter that we replaced the later expression.
     (drop
       (i32.load (i32.const 10))
     )
diff --git a/test/lit/passes/local-cse_all-features.wast b/test/lit/passes/local-cse_all-features.wast
index b51240c50..b5a619997 100644
--- a/test/lit/passes/local-cse_all-features.wast
+++ b/test/lit/passes/local-cse_all-features.wast
@@ -65,8 +65,48 @@
   ;; CHECK:      (type $B (array (mut i32)))
   (type $B (array (mut i32)))
 
+
+  ;; CHECK:      (type $ref?|$A|_=>_none (func (param (ref null $A))))
+
   ;; CHECK:      (type $none_=>_none (func))
 
+  ;; CHECK:      (func $struct-gets-nullable (param $ref (ref null $A))
+  ;; CHECK-NEXT:  (local $1 i32)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (local.tee $1
+  ;; CHECK-NEXT:    (struct.get $A 0
+  ;; CHECK-NEXT:     (local.get $ref)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (local.get $1)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (local.get $1)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $struct-gets-nullable (param $ref (ref null $A))
+    ;; Repeated loads from a struct can be optimized, even with a nullable
+    ;; reference: if we trap, it does not matter that we replaced the later
+    ;; expressions).
+    (drop
+      (struct.get $A 0
+        (local.get $ref)
+      )
+    )
+    (drop
+      (struct.get $A 0
+        (local.get $ref)
+      )
+    )
+    (drop
+      (struct.get $A 0
+        (local.get $ref)
+      )
+    )
+  )
+
   ;; CHECK:      (func $struct-gets (param $ref (ref $A))
   ;; CHECK-NEXT:  (local $1 i32)
   ;; CHECK-NEXT:  (drop
@@ -86,8 +126,8 @@
   (func $struct-gets (param $ref (ref $A))
     ;; Repeated loads from a struct can be optimized.
     ;;
-    ;; Note that these struct.gets cannot trap as the reference is non-nullable,
-    ;; so there are no side effects here, and we can optimize.
+    ;; A potential trap would not stop us (see previous testcase), but here
+    ;; there is also no trap possible anyhow, and we should optimize.
     (drop
       (struct.get $A 0
         (local.get $ref)