Skip to content

Commit 6bc0c8e

Browse files
committed
Bailout from an illegal transformation in semantic arc opts
tryJoinIfDestroyConsumingUseInSameBlock replaces a copy with its operand when there is no use of the copy's operand between the copy's forwarded consuming use and the copy operand's destroy in the same block. It is illegal to do this transformation when there is a non-consuming use of the copy operand after the forwarded consuming use of the copy. The code checking this illegal case was not considerin the case where the consuming use of the copy was in the same instruction as the non-consuming use of the copy operand. rdar://154712867
1 parent bf61199 commit 6bc0c8e

File tree

2 files changed

+48
-1
lines changed

2 files changed

+48
-1
lines changed

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,13 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
503503
if (visitedInsts.count(use->getUser()))
504504
return false;
505505

506+
// If the cvi's operand value has a non-consuming use in the same
507+
// instruction which consumes the copy, bailout. NOTE:
508+
// isUseBetweenInstAndBlockEnd does not check this.
509+
if (user == singleCVIConsumingUse->getUser()) {
510+
return false;
511+
}
512+
506513
// Ok, we have a use that isn't in our visitedInsts region. That being said,
507514
// we may still have a use that introduces a new BorrowScope onto our
508515
// copy_value's operand that overlaps with our forwarding value region. In
@@ -513,14 +520,15 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
513520
// we need to only find scopes that end within the region in between the
514521
// singleConsumingUse (the original forwarded use) and the destroy_value. In
515522
// such a case, we must bail!
516-
if (auto operand = BorrowingOperand(use))
523+
if (auto operand = BorrowingOperand(use)) {
517524
if (!operand.visitScopeEndingUses([&](Operand *endScopeUse) {
518525
// Return false if we did see the relevant end scope instruction
519526
// in the block. That means that we are going to exit early and
520527
// return false.
521528
return !visitedInsts.count(endScopeUse->getUser());
522529
}))
523530
return false;
531+
}
524532
}
525533

526534
// Ok, we now know that we can eliminate this value.

test/SILOptimizer/semantic-arc-opts-lifetime-joining.sil

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,3 +975,42 @@ bb0:
975975
return %r : $()
976976
}
977977

978+
sil @consume_and_borrow : $@convention(thin) (@owned Klass, @guaranteed Klass) -> ()
979+
sil @borrow : $@convention(thin) (@guaranteed Klass) -> ()
980+
sil @consume : $@convention(thin) (@owned Klass) -> ()
981+
sil @get_klass : $@convention(thin) () -> @owned Klass
982+
983+
// CHECK-LABEL: sil hidden [ossa] @borrow_and_consume_copyable_test1 :
984+
// CHECK-NOT: copy_value
985+
// CHECK-LABEL: } // end sil function 'borrow_and_consume_copyable_test1'
986+
sil hidden [ossa] @borrow_and_consume_copyable_test1 : $@convention(thin) () -> () {
987+
bb0:
988+
%0 = function_ref @get_klass : $@convention(thin) () -> @owned Klass
989+
%1 = apply %0() : $@convention(thin) () -> @owned Klass
990+
%2 = move_value [var_decl] %1
991+
%3 = copy_value %2
992+
%4 = function_ref @borrow : $@convention(thin) (@guaranteed Klass) -> ()
993+
%5 = apply %4(%2) : $@convention(thin) (@guaranteed Klass) -> ()
994+
%6 = function_ref @consume : $@convention(thin) (@owned Klass) -> ()
995+
%7 = apply %6(%3) : $@convention(thin) (@owned Klass) -> ()
996+
destroy_value %2
997+
%9 = tuple ()
998+
return %9
999+
}
1000+
1001+
// CHECK-LABEL: sil hidden [ossa] @borrow_and_consume_copyable_test2 :
1002+
// CHECK: copy_value
1003+
// CHECK-LABEL: } // end sil function 'borrow_and_consume_copyable_test2'
1004+
sil hidden [ossa] @borrow_and_consume_copyable_test2 : $@convention(thin) () -> () {
1005+
bb0:
1006+
%0 = function_ref @get_klass : $@convention(thin) () -> @owned Klass
1007+
%1 = apply %0() : $@convention(thin) () -> @owned Klass
1008+
%2 = move_value [var_decl] %1
1009+
%3 = copy_value %2
1010+
%4 = function_ref @consume_and_borrow : $@convention(thin) (@owned Klass, @guaranteed Klass) -> ()
1011+
%5 = apply %4(%3, %2) : $@convention(thin) (@owned Klass, @guaranteed Klass) -> ()
1012+
destroy_value %2
1013+
%7 = tuple ()
1014+
return %7
1015+
}
1016+

0 commit comments

Comments
 (0)