Skip to content

Commit 5b5b55f

Browse files
gottesmmgottesmm
authored
Merge pull request #65842 from gottesmm/more-resilient-stuff
[move-only] Batched resilient changes
2 parents dc73b5a + 81a09b3 commit 5b5b55f

File tree

7 files changed

+294
-1
lines changed

7 files changed

+294
-1
lines changed

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4261,7 +4261,19 @@ static void emitBorrowedLValueRecursive(SILGenFunction &SGF,
42614261
}
42624262
}
42634263

4264+
// TODO: This does not take into account resilience, we should probably use
4265+
// getArgumentType()... but we do not have the SILFunctionType here...
42644266
assert(param.getInterfaceType() == value.getType().getASTType());
4267+
4268+
// If we have an indirect_guaranteed argument, move this using store_borrow
4269+
// into an alloc_stack.
4270+
if (SGF.silConv.useLoweredAddresses() &&
4271+
param.isIndirectInGuaranteed() && value.getType().isObject()) {
4272+
SILValue alloca = SGF.emitTemporaryAllocation(loc, value.getType());
4273+
value = SGF.emitFormalEvaluationManagedStoreBorrow(loc, value.getValue(),
4274+
alloca);
4275+
}
4276+
42654277
args[argIndex++] = value;
42664278
}
42674279

‎lib/SILGen/SILGenBuilder.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,9 +778,19 @@ ManagedValue SILGenBuilder::createStoreBorrow(SILLocation loc,
778778
SILValue address) {
779779
assert(value.getOwnershipKind() == OwnershipKind::Guaranteed);
780780
auto *sbi = createStoreBorrow(loc, value.getValue(), address);
781+
SGF.Cleanups.pushCleanup<EndBorrowCleanup>(sbi);
781782
return ManagedValue(sbi, CleanupHandle::invalid());
782783
}
783784

785+
ManagedValue SILGenBuilder::createFormalAccessStoreBorrow(SILLocation loc,
786+
ManagedValue value,
787+
SILValue address) {
788+
assert(value.getOwnershipKind() == OwnershipKind::Guaranteed);
789+
auto *sbi = createStoreBorrow(loc, value.getValue(), address);
790+
return SGF.emitFormalEvaluationManagedBorrowedRValueWithCleanup(
791+
loc, value.getValue(), sbi);
792+
}
793+
784794
ManagedValue SILGenBuilder::createStoreBorrowOrTrivial(SILLocation loc,
785795
ManagedValue value,
786796
SILValue address) {

‎lib/SILGen/SILGenBuilder.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ class SILGenBuilder : public SILBuilder {
202202
using SILBuilder::createStoreBorrow;
203203
ManagedValue createStoreBorrow(SILLocation loc, ManagedValue value,
204204
SILValue address);
205+
ManagedValue createFormalAccessStoreBorrow(SILLocation loc, ManagedValue value,
206+
SILValue address);
205207

206208
/// Create a store_borrow if we have a non-trivial value and a store [trivial]
207209
/// otherwise.

‎lib/SILGen/SILGenProlog.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,12 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
3939
}
4040

4141
SILValue SILGenFunction::emitSelfDeclForDestructor(VarDecl *selfDecl) {
42+
SILFunctionConventions conventions = F.getConventionsInContext();
43+
4244
// Emit the implicit 'self' argument.
43-
SILType selfType = getLoweredType(selfDecl->getType());
45+
SILType selfType = conventions.getSILArgumentType(
46+
conventions.getNumSILArguments() - 1, F.getTypeExpansionContext());
47+
selfType = F.mapTypeIntoContext(selfType);
4448
SILValue selfValue = F.begin()->createFunctionArgument(selfType, selfDecl);
4549

4650
// If we have a move only type, then mark it with mark_must_check so we can't

‎lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,21 @@ bool swift::siloptimizer::searchForCandidateObjectMarkMustChecks(
110110
}
111111
}
112112

113+
// In the case we have a resilient argument, we may have the following pattern:
114+
//
115+
// bb0(%0 : $*Type): // in_guaranteed
116+
// %1 = load_borrow %0
117+
// %2 = copy_value
118+
// %3 = mark_must_check [no_copy_or_assign]
119+
if (auto *lbi = dyn_cast<LoadBorrowInst>(cvi->getOperand())) {
120+
if (auto *arg = dyn_cast<SILFunctionArgument>(lbi->getOperand())) {
121+
if (arg->getKnownParameterInfo().isIndirectInGuaranteed()) {
122+
moveIntroducersToProcess.insert(mmci);
123+
continue;
124+
}
125+
}
126+
}
127+
113128
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
114129
if (bbi->isLexical()) {
115130
moveIntroducersToProcess.insert(mmci);
@@ -656,6 +671,11 @@ void MoveOnlyObjectCheckerPImpl::check(DominanceInfo *domTree,
656671
i = copyToMoveOnly;
657672
}
658673

674+
// Handle:
675+
//
676+
// bb0(%0 : @guaranteed $Type):
677+
// %1 = copy_value %0
678+
// %2 = mark_must_check [no_consume_or_assign] %1
659679
if (auto *arg = dyn_cast<SILFunctionArgument>(i->getOperand(0))) {
660680
if (arg->getOwnershipKind() == OwnershipKind::Guaranteed) {
661681
for (auto *use : markedInst->getConsumingUses()) {
@@ -669,6 +689,28 @@ void MoveOnlyObjectCheckerPImpl::check(DominanceInfo *domTree,
669689
continue;
670690
}
671691
}
692+
693+
// Handle:
694+
//
695+
// bb0(%0 : $*Type): // in_guaranteed
696+
// %1 = load_borrow %0
697+
// %2 = copy_value %1
698+
// %3 = mark_must_check [no_consume_or_assign] %2
699+
if (auto *lbi = dyn_cast<LoadBorrowInst>(i->getOperand(0))) {
700+
if (auto *arg = dyn_cast<SILFunctionArgument>(lbi->getOperand())) {
701+
if (arg->getKnownParameterInfo().isIndirectInGuaranteed()) {
702+
for (auto *use : markedInst->getConsumingUses()) {
703+
destroys.push_back(cast<DestroyValueInst>(use->getUser()));
704+
}
705+
while (!destroys.empty())
706+
destroys.pop_back_val()->eraseFromParent();
707+
markedInst->replaceAllUsesWith(lbi);
708+
markedInst->eraseFromParent();
709+
cvi->eraseFromParent();
710+
continue;
711+
}
712+
}
713+
}
672714
}
673715
}
674716
}

‎test/SILGen/moveonly_library_evolution.swift‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy -enable-library-evolution %s | %FileCheck %s
2+
3+
////////////////////////
4+
// MARK: Declarations //
5+
////////////////////////
6+
7+
public struct EmptyStruct : ~Copyable {}
8+
public struct NonEmptyStruct : ~Copyable {
9+
var e = EmptyStruct()
10+
}
11+
public class CopyableKlass {
12+
var s = NonEmptyStruct()
13+
14+
let letStruct = NonEmptyStruct()
15+
}
16+
17+
public func borrowVal(_ x: borrowing EmptyStruct) {}
18+
19+
//////////////////////
20+
// MARK: DeinitTest //
21+
//////////////////////
22+
23+
// CHECK-LABEL: sil [ossa] @$s26moveonly_library_evolution10DeinitTestVfD : $@convention(method) (@in DeinitTest) -> () {
24+
// CHECK: bb0([[ARG:%.*]] : $*DeinitTest):
25+
// CHECK: drop_deinit [[ARG]]
26+
// CHECK: } // end sil function '$s26moveonly_library_evolution10DeinitTestVfD'
27+
public struct DeinitTest : ~Copyable {
28+
deinit {
29+
}
30+
}
31+
32+
//////////////////////////////////////////
33+
// MARK: Caller Argument Spilling Tests //
34+
//////////////////////////////////////////
35+
36+
// CHECK-LABEL: sil [ossa] @$s26moveonly_library_evolution29callerArgumentSpillingTestArgyyAA13CopyableKlassCF : $@convention(thin) (@guaranteed CopyableKlass) -> () {
37+
// CHECK: bb0([[ARG:%.*]] : @guaranteed $CopyableKlass):
38+
// CHECK: [[ADDR:%.*]] = ref_element_addr [[ARG]]
39+
// CHECK: [[MARKED_ADDR:%.*]] = mark_must_check [no_consume_or_assign] [[ADDR]]
40+
// CHECK: [[LOADED_VALUE:%.*]] = load [copy] [[MARKED_ADDR]]
41+
// CHECK: [[BORROWED_LOADED_VALUE:%.*]] = begin_borrow [[LOADED_VALUE]]
42+
// CHECK: [[EXT:%.*]] = struct_extract [[BORROWED_LOADED_VALUE]]
43+
// CHECK: [[SPILL:%.*]] = alloc_stack $EmptyStruct
44+
// CHECK: [[STORE_BORROW:%.*]] = store_borrow [[EXT]] to [[SPILL]]
45+
// CHECK: apply {{%.*}}([[STORE_BORROW]]) : $@convention(thin) (@in_guaranteed EmptyStruct) -> ()
46+
// CHECK: end_borrow [[STORE_BORROW]]
47+
// CHECK: end_borrow [[BORROWED_LOADED_VALUE]]
48+
// CHECK: } // end sil function '$s26moveonly_library_evolution29callerArgumentSpillingTestArgyyAA13CopyableKlassCF'
49+
public func callerArgumentSpillingTestArg(_ x: CopyableKlass) {
50+
borrowVal(x.letStruct.e)
51+
}

‎test/SILOptimizer/moveonly_addresschecker_diagnostics_library_evolution.swift‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature NoImplicitCopy -sil-verify-all -verify -enable-library-evolution %s
2+
3+
// This test is used to validate that we properly handle library evolution code
4+
// until we can get all of the normal moveonly_addresschecker_diagnostics test
5+
// case to pass.
6+
7+
////////////////////////
8+
// MARK: Declarations //
9+
////////////////////////
10+
11+
@_moveOnly public struct EmptyStruct {}
12+
@_moveOnly public struct NonEmptyStruct {
13+
var e = EmptyStruct()
14+
}
15+
public class CopyableKlass {
16+
var varS = NonEmptyStruct()
17+
var letS = NonEmptyStruct()
18+
}
19+
20+
public func borrowVal(_ x: borrowing NonEmptyStruct) {}
21+
public func borrowVal(_ x: borrowing EmptyStruct) {}
22+
public func consumeVal(_ x: consuming NonEmptyStruct) {}
23+
public func consumeVal(_ x: consuming EmptyStruct) {}
24+
25+
let copyableKlassLetGlobal = CopyableKlass()
26+
var copyableKlassVarGlobal = CopyableKlass()
27+
28+
/////////////////
29+
// MARK: Tests //
30+
/////////////////
31+
32+
public struct DeinitTest : ~Copyable {
33+
deinit {}
34+
}
35+
36+
public protocol P {}
37+
38+
public struct GenericDeinitTest<T : P> : ~Copyable {
39+
deinit {}
40+
}
41+
42+
//////////////////////////////////////////
43+
// MARK: Caller Argument Let Spill Test //
44+
//////////////////////////////////////////
45+
46+
public func callerBorrowClassLetFieldForArgumentSpillingTestLet() {
47+
let x = CopyableKlass()
48+
borrowVal(x.letS.e)
49+
}
50+
51+
public func callerBorrowClassLetFieldForArgumentSpillingTestVar() {
52+
var x = CopyableKlass()
53+
x = CopyableKlass()
54+
borrowVal(x.letS.e)
55+
}
56+
57+
public func callerBorrowClassLetFieldForArgumentSpillingTestArg(_ x: CopyableKlass) {
58+
borrowVal(x.letS.e)
59+
}
60+
61+
public func callerBorrowClassLetFieldForArgumentSpillingTestInOutArg(_ x: inout CopyableKlass) {
62+
borrowVal(x.letS.e)
63+
}
64+
65+
public func callerBorrowClassLetFieldForArgumentSpillingTestConsumingArg(_ x: consuming CopyableKlass) {
66+
borrowVal(x.letS.e)
67+
}
68+
69+
public func callerBorrowClassLetFieldForArgumentSpillingTestLetGlobal() {
70+
borrowVal(copyableKlassLetGlobal.letS.e)
71+
}
72+
73+
public func callerBorrowClassLetFieldForArgumentSpillingTestVarGlobal() {
74+
borrowVal(copyableKlassVarGlobal.letS.e)
75+
}
76+
77+
public func callerConsumeClassLetFieldForArgumentSpillingTestLet() {
78+
let x = CopyableKlass()
79+
consumeVal(x.letS.e)
80+
}
81+
82+
public func callerConsumeClassLetFieldForArgumentSpillingTestVar() {
83+
var x = CopyableKlass()
84+
x = CopyableKlass()
85+
consumeVal(x.letS.e)
86+
}
87+
88+
public func callerConsumeClassLetFieldForArgumentSpillingTestArg(_ x: CopyableKlass) {
89+
consumeVal(x.letS.e)
90+
}
91+
92+
public func callerConsumeClassLetFieldForArgumentSpillingTestInOutArg(_ x: inout CopyableKlass) {
93+
consumeVal(x.letS.e)
94+
}
95+
96+
public func callerConsumeClassLetFieldForArgumentSpillingTestConsumingArg(_ x: consuming CopyableKlass) {
97+
consumeVal(x.letS.e)
98+
}
99+
100+
public func callerConsumeClassLetFieldForArgumentSpillingTestLetGlobal() {
101+
consumeVal(copyableKlassLetGlobal.letS.e)
102+
}
103+
104+
public func callerConsumeClassLetFieldForArgumentSpillingTestVarGlobal() {
105+
consumeVal(copyableKlassVarGlobal.letS.e)
106+
}
107+
108+
////////////////////
109+
// MARK: Var Test //
110+
////////////////////
111+
112+
public func callerBorrowClassVarFieldForArgumentSpillingTestLet() {
113+
let x = CopyableKlass()
114+
borrowVal(x.varS.e)
115+
}
116+
117+
public func callerBorrowClassVarFieldForArgumentSpillingTestVar() {
118+
var x = CopyableKlass()
119+
x = CopyableKlass()
120+
borrowVal(x.varS.e)
121+
}
122+
123+
public func callerBorrowClassVarFieldForArgumentSpillingTestArg(_ x: CopyableKlass) {
124+
borrowVal(x.varS.e)
125+
}
126+
127+
public func callerBorrowClassVarFieldForArgumentSpillingTestInOutArg(_ x: inout CopyableKlass) {
128+
borrowVal(x.varS.e)
129+
}
130+
131+
public func callerBorrowClassVarFieldForArgumentSpillingTestConsumingArg(_ x: consuming CopyableKlass) {
132+
borrowVal(x.varS.e)
133+
}
134+
135+
public func callerBorrowClassVarFieldForArgumentSpillingTestLetGlobal() {
136+
borrowVal(copyableKlassLetGlobal.varS.e)
137+
}
138+
139+
public func callerBorrowClassVarFieldForArgumentSpillingTestVarGlobal() {
140+
borrowVal(copyableKlassVarGlobal.varS.e)
141+
}
142+
143+
public func callerConsumeClassVarFieldForArgumentSpillingTestLet() {
144+
let x = CopyableKlass()
145+
consumeVal(x.varS.e)
146+
}
147+
148+
public func callerConsumeClassVarFieldForArgumentSpillingTestVar() {
149+
var x = CopyableKlass()
150+
x = CopyableKlass()
151+
consumeVal(x.varS.e)
152+
}
153+
154+
public func callerConsumeClassVarFieldForArgumentSpillingTestArg(_ x: CopyableKlass) {
155+
consumeVal(x.varS.e)
156+
}
157+
158+
public func callerConsumeClassVarFieldForArgumentSpillingTestInOutArg(_ x: inout CopyableKlass) {
159+
consumeVal(x.varS.e)
160+
}
161+
162+
public func callerConsumeClassVarFieldForArgumentSpillingTestConsumingArg(_ x: consuming CopyableKlass) {
163+
consumeVal(x.varS.e)
164+
}
165+
166+
public func callerConsumeClassVarFieldForArgumentSpillingTestLetGlobal() {
167+
consumeVal(copyableKlassLetGlobal.varS.e)
168+
}
169+
170+
public func callerConsumeClassVarFieldForArgumentSpillingTestVarGlobal() {
171+
consumeVal(copyableKlassVarGlobal.varS.e)
172+
}

0 commit comments

Comments
 (0)