Skip to content

Commit 52ac4c1

Browse files
authored
[GC] RemoveUnusedBrs must not un-refine sent types (#7421)
The pass refines cast types of br_on, but that will un-refine the sent type of a br_on_cast_fail - a more refined cast type means more things fail it, so more things are sent. We need to avoid that.
1 parent 05a8c8d commit 52ac4c1

File tree

2 files changed

+71
-6
lines changed

2 files changed

+71
-6
lines changed

src/passes/RemoveUnusedBrs.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,18 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
906906
// further optimizations after this, and those optimizations might even
907907
// benefit from this improvement.
908908
auto glb = Type::getGreatestLowerBound(curr->castType, refType);
909+
if (curr->op == BrOnCastFail) {
910+
// BrOnCastFail sends the input type, with adjusted nullability. The
911+
// input heap type makes sense for the branch target, and we will not
912+
// change it anyhow, but we need to be careful with nullability: if
913+
// the cast type was nullable, then we were sending a non-nullable
914+
// value to the branch, and if we refined the cast type to non-
915+
// nullable, we would no longer be doing that. In other words, we must
916+
// not refine the nullability, as that would *un*refine the send type.
917+
if (curr->castType.isNullable() && glb.isNonNullable()) {
918+
glb = glb.with(Nullable);
919+
}
920+
}
909921
if (glb != Type::unreachable && glb != curr->castType) {
910922
curr->castType = glb;
911923
auto oldType = curr->type;

test/lit/passes/remove-unused-brs-gc.wast

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -494,8 +494,10 @@
494494
;; CHECK-NEXT: )
495495
;; CHECK-NEXT: (drop
496496
;; CHECK-NEXT: (br $block
497-
;; CHECK-NEXT: (local.tee $any
498-
;; CHECK-NEXT: (struct.new_default $struct2)
497+
;; CHECK-NEXT: (ref.as_non_null
498+
;; CHECK-NEXT: (local.tee $any
499+
;; CHECK-NEXT: (struct.new_default $struct2)
500+
;; CHECK-NEXT: )
499501
;; CHECK-NEXT: )
500502
;; CHECK-NEXT: )
501503
;; CHECK-NEXT: )
@@ -531,7 +533,10 @@
531533
)
532534
)
533535
(drop
534-
;; Ditto.
536+
;; Ditto, but also add a ref.as_non_null, as we must keep sending a non-
537+
;; null value to the block (the block would still validate either way, but
538+
;; we do not want to un-refine the sent value). See the next function for a
539+
;; test with a non-nullable block.
535540
(br_on_cast_fail $block anyref (ref null $struct)
536541
(local.tee $any (struct.new $struct2))
537542
)
@@ -552,6 +557,54 @@
552557
)
553558
)
554559

560+
;; CHECK: (func $br_on_cast_fail_unrelated-fallthrough-non-null (type $11) (result (ref any))
561+
;; CHECK-NEXT: (local $any anyref)
562+
;; CHECK-NEXT: (local $nullable-struct2 (ref null $struct2))
563+
;; CHECK-NEXT: (block $block (result (ref any))
564+
;; CHECK-NEXT: (drop
565+
;; CHECK-NEXT: (br $block
566+
;; CHECK-NEXT: (ref.as_non_null
567+
;; CHECK-NEXT: (local.tee $any
568+
;; CHECK-NEXT: (struct.new_default $struct2)
569+
;; CHECK-NEXT: )
570+
;; CHECK-NEXT: )
571+
;; CHECK-NEXT: )
572+
;; CHECK-NEXT: )
573+
;; CHECK-NEXT: (drop
574+
;; CHECK-NEXT: (block (result nullref)
575+
;; CHECK-NEXT: (br_on_non_null $block
576+
;; CHECK-NEXT: (local.tee $any
577+
;; CHECK-NEXT: (local.get $nullable-struct2)
578+
;; CHECK-NEXT: )
579+
;; CHECK-NEXT: )
580+
;; CHECK-NEXT: (ref.null none)
581+
;; CHECK-NEXT: )
582+
;; CHECK-NEXT: )
583+
;; CHECK-NEXT: (unreachable)
584+
;; CHECK-NEXT: )
585+
;; CHECK-NEXT: )
586+
(func $br_on_cast_fail_unrelated-fallthrough-non-null (result (ref any))
587+
;; Same as above, but the block is now non-nullable. Only the branches that
588+
;; work with that are tested.
589+
(local $any anyref)
590+
(local $nullable-struct2 (ref null $struct2))
591+
(block $block (result (ref any)) ;; this changed, and the function's results
592+
(drop
593+
;; Will definitely take the branch.
594+
(br_on_cast_fail $block anyref (ref null $struct)
595+
(local.tee $any (struct.new $struct2))
596+
)
597+
)
598+
(drop
599+
;; Still has to do a null check.
600+
(br_on_cast_fail $block anyref (ref null $struct)
601+
(local.tee $any (local.get $nullable-struct2))
602+
)
603+
)
604+
(unreachable)
605+
)
606+
)
607+
555608
;; CHECK: (func $br_on_cast-unreachable (type $7) (param $i31ref i31ref) (result anyref)
556609
;; CHECK-NEXT: (block $block
557610
;; CHECK-NEXT: (drop
@@ -840,7 +893,7 @@
840893
)
841894
)
842895

843-
;; CHECK: (func $threading (type $11) (param $x anyref)
896+
;; CHECK: (func $threading (type $12) (param $x anyref)
844897
;; CHECK-NEXT: (block $outer
845898
;; CHECK-NEXT: (block $inner
846899
;; CHECK-NEXT: (drop
@@ -864,7 +917,7 @@
864917
)
865918
)
866919

867-
;; CHECK: (func $test (type $12) (param $x (ref any))
920+
;; CHECK: (func $test (type $13) (param $x (ref any))
868921
;; CHECK-NEXT: (local $temp anyref)
869922
;; CHECK-NEXT: (drop
870923
;; CHECK-NEXT: (block $block (result (ref $struct-nn))
@@ -916,7 +969,7 @@
916969
)
917970
)
918971

919-
;; CHECK: (func $select-refinalize (type $13) (param $param (ref $struct)) (result (ref struct))
972+
;; CHECK: (func $select-refinalize (type $14) (param $param (ref $struct)) (result (ref struct))
920973
;; CHECK-NEXT: (select (result (ref $struct))
921974
;; CHECK-NEXT: (select (result (ref $struct))
922975
;; CHECK-NEXT: (global.get $struct)

0 commit comments

Comments
 (0)