Never flip a call from unreachable to reachable during inlining (#5493)

See example in the new comment. In general, we never want to go from
unreachable to reachable (refinalize doesn't even try), as it misses out on
DCE opportunities. Also it may have validation issues, which is how the
fuzzer found this.

Remove code in the same place that was redundant after the refinalize
was added in #5492. That simplifies some existing testcases slightly, by
removing an unneeded br we added, and now we add a new unreachable
at the end in other cases that need it.

5 files changed, 68 insertions, 8 deletions

diff --git a/test/lit/passes/inlining-unreachable.wast b/test/lit/passes/inlining-unreachable.wast
index e8797d5b9..66ed2097c 100644
--- a/test/lit/passes/inlining-unreachable.wast
+++ b/test/lit/passes/inlining-unreachable.wast
@@ -16,7 +16,6 @@
   ;; CHECK:      (func $call-trap (type $none_=>_none)
   ;; CHECK-NEXT:  (block $__inlined_func$trap
   ;; CHECK-NEXT:   (unreachable)
-  ;; CHECK-NEXT:   (br $__inlined_func$trap)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $call-trap
@@ -59,7 +58,6 @@
   ;; CHECK-NEXT:    (nop)
   ;; CHECK-NEXT:    (unreachable)
   ;; CHECK-NEXT:   )
-  ;; CHECK-NEXT:   (br $__inlined_func$contents-then-trap)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $call-contents-then-trap
diff --git a/test/lit/passes/inlining_all-features.wast b/test/lit/passes/inlining_all-features.wast
index d26a2aa9f..eee8fbdc9 100644
--- a/test/lit/passes/inlining_all-features.wast
+++ b/test/lit/passes/inlining_all-features.wast
@@ -142,13 +142,11 @@
  ;; CHECK:      (func $1 (type $none_=>_none)
  ;; CHECK-NEXT:  (block $__inlined_func$0
  ;; CHECK-NEXT:   (unreachable)
- ;; CHECK-NEXT:   (br $__inlined_func$0)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  ;; NOMNL:      (func $1 (type $none_=>_none)
  ;; NOMNL-NEXT:  (block $__inlined_func$0
  ;; NOMNL-NEXT:   (unreachable)
- ;; NOMNL-NEXT:   (br $__inlined_func$0)
  ;; NOMNL-NEXT:  )
  ;; NOMNL-NEXT: )
  (func $1
diff --git a/test/lit/passes/inlining_enable-tail-call.wast b/test/lit/passes/inlining_enable-tail-call.wast
index ed642594c..34b230371 100644
--- a/test/lit/passes/inlining_enable-tail-call.wast
+++ b/test/lit/passes/inlining_enable-tail-call.wast
@@ -561,7 +561,6 @@
  ;; CHECK-NEXT:     (br $__inlined_func$1)
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
- ;; CHECK-NEXT:   (br $__inlined_func$1)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  (func $0
@@ -629,7 +628,6 @@
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:    (br $__inlined_func$1)
  ;; CHECK-NEXT:   )
- ;; CHECK-NEXT:   (br $__inlined_func$1)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
  (func $0
@@ -694,7 +692,6 @@
  ;; CHECK-NEXT:       )
  ;; CHECK-NEXT:      )
  ;; CHECK-NEXT:     )
- ;; CHECK-NEXT:     (br $__inlined_func$13)
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
diff --git a/test/lit/passes/inlining_optimize-level=3.wast b/test/lit/passes/inlining_optimize-level=3.wast
index e5a0bc7e1..9e20e27dd 100644
--- a/test/lit/passes/inlining_optimize-level=3.wast
+++ b/test/lit/passes/inlining_optimize-level=3.wast
@@ -495,3 +495,71 @@
   (unreachable)
  )
 )
+
+;; We inline multiple times here, and in the sequence of those inlinings we
+;; turn the code in $B unreachable (when we inline $D), and no later inlining
+;; (of $C or $A, or even $C's inlining in $A) should turn it into anything else
+;; than an unreachable - once it is unreachable, we should keep it that way.
+;; (That avoids possible validation problems, and maximizes DCE.) To keep it
+;; unreachable we'll add an unreachable instruction after the inlined code.
+(module
+ ;; CHECK:      (type $f32_=>_none (func (param f32)))
+
+ ;; CHECK:      (type $none_=>_none (func))
+
+ ;; CHECK:      (func $A (param $0 f32)
+ ;; CHECK-NEXT:  (local $1 f32)
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (block (result f32)
+ ;; CHECK-NEXT:    (block $__inlined_func$C (result f32)
+ ;; CHECK-NEXT:     (local.set $1
+ ;; CHECK-NEXT:      (local.get $0)
+ ;; CHECK-NEXT:     )
+ ;; CHECK-NEXT:     (local.get $1)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $A (param $0 f32)
+  (drop
+   (call $C
+    (local.get $0)
+   )
+  )
+ )
+ ;; CHECK:      (func $B
+ ;; CHECK-NEXT:  (local $0 f32)
+ ;; CHECK-NEXT:  (call $A
+ ;; CHECK-NEXT:   (block
+ ;; CHECK-NEXT:    (block
+ ;; CHECK-NEXT:     (drop
+ ;; CHECK-NEXT:      (block $__inlined_func$C (result f32)
+ ;; CHECK-NEXT:       (local.tee $0
+ ;; CHECK-NEXT:        (block
+ ;; CHECK-NEXT:         (block $__inlined_func$D
+ ;; CHECK-NEXT:          (unreachable)
+ ;; CHECK-NEXT:         )
+ ;; CHECK-NEXT:        )
+ ;; CHECK-NEXT:       )
+ ;; CHECK-NEXT:       (local.get $0)
+ ;; CHECK-NEXT:      )
+ ;; CHECK-NEXT:     )
+ ;; CHECK-NEXT:     (unreachable)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $B
+  (call $A
+   (call $C
+    (call $D)
+   )
+  )
+ )
+ (func $C (param $0 f32) (result f32)
+  (local.get $0)
+ )
+ (func $D (result f32)
+  (unreachable)
+ )
+)
diff --git a/test/lit/passes/inlining_splitting.wast b/test/lit/passes/inlining_splitting.wast
index 2f840a494..9764aa235 100644
--- a/test/lit/passes/inlining_splitting.wast
+++ b/test/lit/passes/inlining_splitting.wast
@@ -231,7 +231,6 @@
   ;; CHECK-NEXT:     (br $l)
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
-  ;; CHECK-NEXT:   (br $__inlined_func$nondefaultable-param)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $call-nondefaultable-param