Fix a bug optimizing out br_on_cast (#5403)

We were considering casts between unrelated types as unconditionally failing,
but in the case where the unrelated types are nullable, the cast could still
succeed if the value is null.

This bug was introduced in #5397.

2 files changed, 102 insertions, 7 deletions

diff --git a/src/ir/gc-type-utils.h b/src/ir/gc-type-utils.h
index 6ac604ad2..8832d4d0f 100644
--- a/src/ir/gc-type-utils.h
+++ b/src/ir/gc-type-utils.h
@@ -61,9 +61,13 @@ inline EvaluationResult evaluateKindCheck(Expression* curr) {
         if (Type::isSubType(br->ref->type, br->castType)) {
           return flip ? Failure : Success;
         }
-        // If the cast type is unrelated to the type we have, the cast will
-        // certainly fail.
-        if (!Type::isSubType(br->castType, br->ref->type)) {
+        // If the cast type is unrelated to the type we have and it's not
+        // possible for the cast to succeed anyway because the value is null,
+        // then the cast will certainly fail. TODO: This is essentially the same
+        // as `canBeCastTo` in OptimizeInstructions. Find a way to deduplicate
+        // this logic.
+        if (!Type::isSubType(br->castType, br->ref->type) &&
+            (br->castType.isNonNullable() || br->ref->type.isNonNullable())) {
           return flip ? Success : Failure;
         }
         return Unknown;
diff --git a/test/lit/passes/remove-unused-brs-gc.wast b/test/lit/passes/remove-unused-brs-gc.wast
index 5c45b9e7c..b4c529f74 100644
--- a/test/lit/passes/remove-unused-brs-gc.wast
+++ b/test/lit/passes/remove-unused-brs-gc.wast
@@ -144,16 +144,29 @@
   )
  )
 
- ;; CHECK:      (func $br_on_cast_unrelated (type $none_=>_ref|$struct|) (result (ref $struct))
- ;; CHECK-NEXT:  (block $block
+ ;; CHECK:      (func $br_on_cast_unrelated (type $none_=>_ref?|$struct|) (result (ref null $struct))
+ ;; CHECK-NEXT:  (local $nullable-struct2 (ref null $struct2))
+ ;; CHECK-NEXT:  (block $block (result (ref null $struct))
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (struct.new_default $struct2)
+ ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:   (drop
  ;; CHECK-NEXT:    (struct.new_default $struct2)
  ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (local.get $nullable-struct2)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (br_on_cast $block null $struct
+ ;; CHECK-NEXT:     (local.get $nullable-struct2)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:   (unreachable)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- (func $br_on_cast_unrelated (result (ref $struct))
-  (block $block (result (ref $struct))
+ (func $br_on_cast_unrelated (result (ref null $struct))
+  (local $nullable-struct2 (ref null $struct2))
+  (block $block (result (ref null $struct))
    (drop
     ;; This cast can be computed at compile time: it will definitely fail, so we
     ;; can remove it.
@@ -161,6 +174,84 @@
      (struct.new $struct2)
     )
    )
+   (drop
+    ;; We can still remove it even if the cast allows nulls.
+    (br_on_cast $block null $struct
+     (struct.new $struct2)
+    )
+   )
+   (drop
+    ;; Or if the cast does not allow nulls and the value is nullable.
+    (br_on_cast $block $struct
+     (local.get $nullable-struct2)
+    )
+   )
+   (drop
+    ;; But if both are nullable, then we can't optimize because the cast would
+    ;; succeed if the value is a null.
+    (br_on_cast $block null $struct
+     (local.get $nullable-struct2)
+    )
+   )
+   (unreachable)
+  )
+ )
+
+ ;; CHECK:      (func $br_on_cast_fail_unrelated (type $none_=>_anyref) (result anyref)
+ ;; CHECK-NEXT:  (local $nullable-struct2 (ref null $struct2))
+ ;; CHECK-NEXT:  (block $block (result (ref null $struct2))
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (br $block
+ ;; CHECK-NEXT:     (struct.new_default $struct2)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (br $block
+ ;; CHECK-NEXT:     (struct.new_default $struct2)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (br $block
+ ;; CHECK-NEXT:     (local.get $nullable-struct2)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (br_on_cast_fail $block null $struct
+ ;; CHECK-NEXT:     (local.get $nullable-struct2)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:   (unreachable)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $br_on_cast_fail_unrelated (result anyref)
+  (local $nullable-struct2 (ref null $struct2))
+  (block $block (result anyref)
+   (drop
+    ;; This cast can be computed at compile time: it will definitely fail, so we
+    ;; can replace it with an unconditional br.
+    (br_on_cast_fail $block $struct
+     (struct.new $struct2)
+    )
+   )
+   (drop
+    ;; We can still replace it even if the cast allows nulls.
+    (br_on_cast_fail $block null $struct
+     (struct.new $struct2)
+    )
+   )
+   (drop
+    ;; Or if the cast does not allow nulls and the value is nullable.
+    (br_on_cast_fail $block $struct
+     (local.get $nullable-struct2)
+    )
+   )
+   (drop
+    ;; But if both are nullable, then we can't optimize because the cast would
+    ;; succeed if the value is a null.
+    (br_on_cast_fail $block null $struct
+     (local.get $nullable-struct2)
+    )
+   )
    (unreachable)
   )
  )