Skip to content

Commit 5ab243e

Browse files
committed
Fix CheckedCastBrJumpThreading when we have dominating checked_cast_br on failure path only
In this case, we know the checked_cast_br will have a false outcome. It is sufficient to transform checked_cast_br into a br and delete the success path. Before this, we were trying to do an OSSA rauw the successarg which is incorrect.
1 parent e5d2475 commit 5ab243e

File tree

2 files changed

+65
-3
lines changed

2 files changed

+65
-3
lines changed

lib/SILOptimizer/Utils/CheckedCastBrJumpThreading.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ SILValue CheckedCastBrJumpThreading::isArgValueEquivalentToCondition(
253253
// Return false if an ownership RAUW is necessary but cannot be performed.
254254
bool CheckedCastBrJumpThreading::Edit::
255255
canRAUW(OwnershipFixupContext &rauwContext) {
256-
if (InvertSuccess || SuccessPreds.empty())
256+
if (InvertSuccess || (SuccessPreds.empty() && !hasUnknownPreds))
257257
return true;
258258

259259
auto *ccbi = cast<CheckedCastBranchInst>(CCBBlock->getTerminator());
@@ -334,7 +334,7 @@ void CheckedCastBrJumpThreading::Edit::modifyCFGForSuccessPreds(
334334

335335
auto *checkedCastBr = cast<CheckedCastBranchInst>(CCBBlock->getTerminator());
336336
auto *oldSuccessArg = checkedCastBr->getSuccessBB()->getArgument(0);
337-
if (InvertSuccess) {
337+
if (InvertSuccess || (SuccessPreds.empty() && !hasUnknownPreds)) {
338338
assert(!hasUnknownPreds && "is not handled, should have been checked");
339339
// This success path is unused, so undef its uses and delete the cast.
340340
oldSuccessArg->replaceAllUsesWithUndef();

test/SILOptimizer/simplify_cfg_checkcast.sil

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ class Klass {
2121
protocol OtherKlass : AnyObject {}
2222

2323
sil [ossa] @consume_klass : $@convention(thin) (@owned Klass) -> ()
24-
sil [ossa] @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
2524
sil [ossa] @get_klass : $@convention(thin) () -> @owned Klass
2625

2726
sil [ossa] @unknown : $@convention(thin) () -> ()
@@ -677,6 +676,69 @@ bb8:
677676
return %25 : $()
678677
}
679678

679+
// CHECK-LABEL: sil [ossa] @redundant_checked_cast_failure_path :
680+
// CHECK: checked_cast_br
681+
// CHECK-NOT: checked_cast_br
682+
// CHECK-LABEL: } // end sil function 'redundant_checked_cast_failure_path'
683+
sil [ossa] @redundant_checked_cast_failure_path : $@convention(method) (@owned Base) -> () {
684+
bb0(%0 : @owned $Base):
685+
%borrow = begin_borrow %0 : $Base
686+
checked_cast_br %borrow : $Base to Derived, bb1, bb2
687+
688+
bb1(%succ1 : @guaranteed $Derived):
689+
br bbexit
690+
691+
bb2(%5 : @guaranteed $Base):
692+
checked_cast_br %5 : $Base to Derived, bb3, bb4
693+
694+
bb3(%9 : @guaranteed $Derived):
695+
br bbexit
696+
697+
bb4(%10 : @guaranteed $Base):
698+
%m = class_method %10 : $Base, #Base.inner : (Base) -> () -> (), $@convention(method) (@guaranteed Base) -> ()
699+
apply %m(%10) : $@convention(method) (@guaranteed Base) -> ()
700+
br bbexit
701+
702+
bbexit:
703+
end_borrow %borrow : $Base
704+
destroy_value %0 : $Base
705+
%t = tuple ()
706+
return %t : $()
707+
}
708+
709+
// CHECK-LABEL: sil [ossa] @redundant_checked_cast_failure_path_not_idom :
710+
// CHECK: checked_cast_br
711+
// CHECK-NOT: checked_cast_br
712+
// CHECK-LABEL: } // end sil function 'redundant_checked_cast_failure_path_not_idom'
713+
sil [ossa] @redundant_checked_cast_failure_path_not_idom : $@convention(method) (@owned Base) -> () {
714+
bb0(%0 : @owned $Base):
715+
%borrow = begin_borrow %0 : $Base
716+
checked_cast_br %borrow : $Base to Derived, bb1, bb2
717+
718+
bb1(%succ1 : @guaranteed $Derived):
719+
br bbexit
720+
721+
bb2(%5 : @guaranteed $Base):
722+
br bb3(%5 : $Base)
723+
724+
bb3(%6 : @guaranteed $Base):
725+
checked_cast_br %6 : $Base to Derived, bb4, bb5
726+
727+
bb4(%9 : @guaranteed $Derived):
728+
br bbexit
729+
730+
bb5(%10 : @guaranteed $Base):
731+
%m = class_method %10 : $Base, #Base.inner : (Base) -> () -> (), $@convention(method) (@guaranteed Base) -> ()
732+
apply %m(%10) : $@convention(method) (@guaranteed Base) -> ()
733+
br bbexit
734+
735+
bbexit:
736+
end_borrow %borrow : $Base
737+
destroy_value %0 : $Base
738+
%t = tuple ()
739+
return %t : $()
740+
}
741+
680742
//!!!TODO: test replacing a guaranteed value with an ownedvalue
681743
// test borrowOverValue->borrowCopyOverScope path
682744

0 commit comments

Comments
 (0)