Skip to content

Commit 6f7d420

Browse files
meg-guptameg-gupta
committed
Fix unchecked_conversion insertion for borrow accessors
1 parent 0a8a3bc commit 6f7d420

File tree

2 files changed

+78
-17
lines changed

2 files changed

+78
-17
lines changed

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5457,9 +5457,20 @@ ManagedValue CallEmission::applyBorrowMutateAccessor() {
54575457
// begin_borrow instructions added for move-only self argument.
54585458
if (selfArgMV.getValue()->getType().isMoveOnly() &&
54595459
selfArgMV.getValue()->getType().isObject()) {
5460-
uncurriedArgs[uncurriedArgs.size() - 1] =
5461-
ManagedValue::forBorrowedObjectRValue(
5462-
lookThroughMoveOnlyCheckerPattern(selfArgMV.getValue()));
5460+
uncurriedArgs.back() = ManagedValue::forBorrowedObjectRValue(
5461+
lookThroughMoveOnlyCheckerPattern(selfArgMV.getValue()));
5462+
}
5463+
5464+
if (fnValue.getFunction()->getConventions().hasGuaranteedResult()) {
5465+
if (isa<LoadBorrowInst>(selfArgMV)) {
5466+
// unchecked_ownership is used to silence the ownership verifier for
5467+
// returning a value produced within a load_borrow scope. SILGenCleanup
5468+
// eliminates it and introduces return_borrow appropriately.
5469+
uncurriedArgs.back() =
5470+
ManagedValue::forForwardedRValue(
5471+
SGF, SGF.B.createUncheckedOwnership(uncurriedLoc.value(),
5472+
selfArgMV.getValue()));
5473+
}
54635474
}
54645475

54655476
auto value = SGF.applyBorrowMutateAccessor(
@@ -5475,22 +5486,13 @@ ManagedValue SILGenFunction::applyBorrowMutateAccessor(
54755486
ApplyOptions options) {
54765487
// Emit the call.
54775488
SmallVector<SILValue, 4> rawResults;
5489+
54785490
emitRawApply(*this, loc, fn, subs, args, substFnType, options,
54795491
/*indirect results*/ {}, /*indirect errors*/ {}, rawResults,
54805492
ExecutorBreadcrumb());
54815493
assert(rawResults.size() == 1);
54825494
auto rawResult = rawResults[0];
54835495

5484-
if (fn.getFunction()->getConventions().hasGuaranteedResult()) {
5485-
auto selfArg = args.back().getValue();
5486-
if (isa<LoadBorrowInst>(selfArg)) {
5487-
// unchecked_ownership is used to silence the ownership verifier for
5488-
// returning a value produced within a load_borrow scope. SILGenCleanup
5489-
// eliminates it and introduces return_borrow appropriately.
5490-
rawResult = B.createUncheckedOwnership(loc, rawResult);
5491-
}
5492-
}
5493-
54945496
if (rawResult->getType().isMoveOnly()) {
54955497
if (rawResult->getType().isAddress()) {
54965498
SILFunctionConventions substFnConv(substFnType, SGM.M);

‎test/SILGen/borrow_accessor.swift‎

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN:%target-swift-frontend -emit-silgen %s -enable-experimental-feature BorrowAndMutateAccessors | %FileCheck %s
2+
// RUN:%target-swift-frontend -c %s -enable-experimental-feature BorrowAndMutateAccessors -Xllvm -sil-print-after=SILGenCleanup 2>&1 | %FileCheck %s --check-prefixes=CHECK-SIL
23

34
// REQUIRES: swift_feature_BorrowAndMutateAccessors
45

@@ -496,6 +497,14 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
496497
// CHECK: return [[REG4]]
497498
// CHECK: }
498499

500+
// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1sAA1SVvb : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed S {
501+
// CHECK-SIL: bb0([[REG0:%.*]] : $*GenWrapper<T>):
502+
// CHECK-SIL: debug_value [[REG0]], let, name "self", argno 1, expr op_deref
503+
// CHECK-SIL: [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._s
504+
// CHECK-SIL: [[REG3:%.*]] = load_borrow [[REG2]]
505+
// CHECK-SIL: return_borrow [[REG3]] from_scopes ([[REG3]])
506+
// CHECK-SIL: }
507+
499508
// CHECK: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1sAA1SVvz : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout S {
500509
// CHECK:bb0([[REG0:%.*]] : $*GenWrapper<T>):
501510
// CHECK: debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -513,6 +522,14 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
513522
// CHECK: return [[REG4]]
514523
// CHECK: }
515524

525+
// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1kAA5KlassCvb : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed Klass {
526+
// CHECK-SIL: bb0([[REG0:%.*]] : $*GenWrapper<T>):
527+
// CHECK-SIL: debug_value [[REG0]], let, name "self", argno 1, expr op_deref
528+
// CHECK-SIL: [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._k
529+
// CHECK-SIL: [[REG3:%.*]] = load_borrow [[REG2]]
530+
// CHECK-SIL: return_borrow [[REG3]] from_scopes ([[REG3]])
531+
// CHECK-SIL: }
532+
516533
// CHECK: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1kAA5KlassCvz : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout Klass {
517534
// CHECK:bb0([[REG0:%.*]] : $*GenWrapper<T>):
518535
// CHECK: debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -552,12 +569,22 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
552569
// CHECK: [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._s
553570
// CHECK: [[REG3:%.*]] = load_borrow [[REG2]]
554571
// CHECK: [[REG4:%.*]] = function_ref @$s15borrow_accessor1SV1kAA5KlassCvb : $@convention(method) (@guaranteed S) -> @guaranteed Klass
555-
// CHECK: [[REG5:%.*]] = apply [[REG4]]([[REG3]]) : $@convention(method) (@guaranteed S) -> @guaranteed Klass
556-
// CHECK: [[REG6:%.*]] = unchecked_ownership [[REG5]]
572+
// CHECK: [[REG5:%.*]] = unchecked_ownership [[REG3]]
573+
// CHECK: [[REG6:%.*]] = apply [[REG4]]([[REG5]]) : $@convention(method) (@guaranteed S) -> @guaranteed Klass
557574
// CHECK: end_borrow [[REG3]]
558575
// CHECK: return [[REG6]]
559576
// CHECK: }
560577

578+
// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV9nested_k1AA5KlassCvb : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed Klass {
579+
// CHECK-SIL: bb0([[REG0:%.*]] : $*GenWrapper<T>):
580+
// CHECK-SIL: debug_value [[REG0]], let, name "self", argno 1, expr op_deref
581+
// CHECK-SIL: [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._s
582+
// CHECK-SIL: [[REG3:%.*]] = load_borrow [[REG2]]
583+
// CHECK-SIL: [[REG4:%.*]] = function_ref @$s15borrow_accessor1SV1kAA5KlassCvb : $@convention(method) (@guaranteed S) -> @guaranteed Klass
584+
// CHECK-SIL: [[REG5:%.*]] = apply [[REG4]]([[REG3]]) : $@convention(method) (@guaranteed S) -> @guaranteed Klass
585+
// CHECK-SIL: return_borrow [[REG5]] from_scopes ([[REG3]])
586+
// CHECK-SIL: }
587+
561588
// CHECK: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV9nested_k1AA5KlassCvz : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout Klass {
562589
// CHECK:bb0([[REG0:%.*]] : $*GenWrapper<T>):
563590
// CHECK: debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -654,6 +681,15 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
654681
// CHECK: return [[REG5]]
655682
// CHECK: }
656683

684+
// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE2ncAA2NCVvb : $@convention(method) <T where T : ~Copyable> (@in_guaranteed GenNCWrapper<T>) -> @guaranteed NC {
685+
// CHECK-SIL: bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
686+
// CHECK-SIL: debug_value [[REG0]], let, name "self", argno 1, expr op_deref
687+
// CHECK-SIL: [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG0]]
688+
// CHECK-SIL: [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._nc
689+
// CHECK-SIL: [[REG4:%.*]] = load_borrow [[REG3]]
690+
// CHECK-SIL: return_borrow [[REG4]] from_scopes ([[REG4]])
691+
// CHECK-SIL: }
692+
657693
// CHECK: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE2ncAA2NCVvz : $@convention(method) <T where T : ~Copyable> (@inout GenNCWrapper<T>) -> @inout NC {
658694
// CHECK:bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
659695
// CHECK: debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -673,6 +709,15 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
673709
// CHECK: return [[REG5]]
674710
// CHECK: }
675711

712+
// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE3ncwAA0D0Vvb : $@convention(method) <T where T : ~Copyable> (@in_guaranteed GenNCWrapper<T>) -> @guaranteed NCWrapper {
713+
// CHECK-SIL: bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
714+
// CHECK-SIL: debug_value [[REG0]], let, name "self", argno 1, expr op_deref
715+
// CHECK-SIL: [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG0]]
716+
// CHECK-SIL: [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw
717+
// CHECK-SIL: [[REG4:%.*]] = load_borrow [[REG3]]
718+
// CHECK-SIL: return_borrow [[REG4]] from_scopes ([[REG4]])
719+
// CHECK-SIL: }
720+
676721
// CHECK: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE3ncwAA0D0Vvz : $@convention(method) <T where T : ~Copyable> (@inout GenNCWrapper<T>) -> @inout NCWrapper {
677722
// CHECK:bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
678723
// CHECK: debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -730,8 +775,8 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
730775
// CHECK: [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw
731776
// CHECK: [[REG4:%.*]] = load_borrow [[REG3]]
732777
// CHECK: [[REG5:%.*]] = function_ref @$s15borrow_accessor9NCWrapperV2ncAA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
733-
// CHECK: [[REG6:%.*]] = apply [[REG5]]([[REG4]]) : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
734-
// CHECK: [[REG7:%.*]] = unchecked_ownership [[REG6]]
778+
// CHECK: [[REG6:%.*]] = unchecked_ownership [[REG4]]
779+
// CHECK: [[REG7:%.*]] = apply [[REG5]]([[REG6]]) : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
735780
// CHECK: [[REG9:%.*]] = copy_value [[REG7]]
736781
// CHECK: [[REG10:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG9]]
737782
// CHECK: [[REG11:%.*]] = begin_borrow [[REG10]]
@@ -741,6 +786,20 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
741786
// CHECK: return [[REG7]]
742787
// CHECK: }
743788

789+
// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE10nested_nc1AA2NCVvb : $@convention(method) <T where T : ~Copyable> (@in_guaranteed GenNCWrapper<T>) -> @guaranteed NC {
790+
// CHECK-SIL: bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
791+
// CHECK-SIL: debug_value [[REG0]], let, name "self", argno 1, expr op_deref
792+
// CHECK-SIL: [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG0]]
793+
// CHECK-SIL: [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw
794+
// CHECK-SIL: [[REG4:%.*]] = load_borrow [[REG3]]
795+
// CHECK-SIL: [[REG5:%.*]] = function_ref @$s15borrow_accessor9NCWrapperV2ncAA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
796+
// CHECK-SIL: [[REG6:%.*]] = apply [[REG5]]([[REG4]]) : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
797+
// CHECK-SIL: [[REG7:%.*]] = copy_value [[REG6]]
798+
// CHECK-SIL: [[REG8:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG7]]
799+
// CHECK-SIL: destroy_value [[REG8]]
800+
// CHECK-SIL: return_borrow [[REG6]] from_scopes ([[REG4]])
801+
// CHECK-SIL: }
802+
744803
// CHECK: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE10nested_nc1AA2NCVvz : $@convention(method) <T where T : ~Copyable> (@inout GenNCWrapper<T>) -> @inout NC {
745804
// CHECK:bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
746805
// CHECK: debug_value [[REG0]], var, name "self", argno 1, expr op_deref

0 commit comments

Comments
 (0)