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.

Author: kripken

src/passes/LocalCSE.cpp

@@ -422,6 +422,15 @@ struct Checker
       // away repeated apperances if it has any.
       EffectAnalyzer effects(options, getModule()->features, curr);
 
+      // We can ignore traps here, as we replace a repeating expression with a
+      // single appearance of it, a store to a local, and gets in the other
+      // locations, and so if the expression traps then the first appearance -
+      // that we keep around - would trap, and the others are never reached
+      // anyhow. (The other checks we perform here, including invalidation and
+      // determinism, will ensure that either all of the appearances trap, or
+      // none of them.)
+      effects.trap = false;
+
       // We also cannot optimize away something that is intrinsically
       // nondeterministic: even if it has no side effects, if it may return a
       // different result each time, then we cannot optimize away repeats.

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))
     )

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)