Skip to content

Commit 251becb

Browse files
jckarterjckarter
authored
Merge pull request #69263 from jckarter/move-only-loadable-to-address-reabstraction
Move only loadable to address reabstraction
2 parents 5a2d1f4 + f3ef432 commit 251becb

File tree

5 files changed

+101
-237
lines changed

5 files changed

+101
-237
lines changed

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6524,9 +6524,9 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
65246524
if (selfParam.isConsumed() && !base.hasCleanup()) {
65256525
base = base.copyUnmanaged(SGF, loc);
65266526
}
6527-
6528-
// If the parameter is indirect, we need to drop the value into
6529-
// temporary memory.
6527+
// If the parameter is indirect, we'll need to drop the value into
6528+
// temporary memory. Make a copy scoped to the current formal access that
6529+
// we can materialize later.
65306530
if (SGF.silConv.isSILIndirect(selfParam)) {
65316531
// It's a really bad idea to materialize when we're about to
65326532
// pass a value to an inout argument, because it's a really easy
@@ -6536,14 +6536,17 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
65366536
// itself).
65376537
assert(!selfParam.isIndirectMutating() &&
65386538
"passing unmaterialized r-value as inout argument");
6539-
6540-
base = base.formallyMaterialize(SGF, loc);
6541-
auto shouldTake = IsTake_t(base.hasCleanup());
6542-
base = SGF.emitFormalAccessLoad(loc, base.forward(SGF),
6543-
SGF.getTypeLowering(baseLoweredType),
6544-
SGFContext(), shouldTake);
6539+
6540+
// Avoid copying the base if it's move-only. It should be OK to do in this
6541+
// case since the move-only base will be accessed in a formal access scope
6542+
// as well. This isn't always true for copyable bases so be less aggressive
6543+
// in that case.
6544+
if (base.getType().isMoveOnly()) {
6545+
base = base.formalAccessBorrow(SGF, loc);
6546+
} else {
6547+
base = base.formalAccessCopy(SGF, loc);
6548+
}
65456549
}
6546-
65476550
return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
65486551
}
65496552

‎lib/SILGen/SILGenLValue.cpp‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2935,10 +2935,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
29352935
SILGenBorrowedBaseVisitor(SILGenLValue &SGL, SILGenFunction &SGF)
29362936
: SGL(SGL), SGF(SGF) {}
29372937

2938-
/// Returns the subexpr
29392938
static bool isNonCopyableBaseBorrow(SILGenFunction &SGF, Expr *e) {
2940-
if (auto *le = dyn_cast<LoadExpr>(e))
2941-
return le->getType()->isNoncopyable();
29422939
if (auto *m = dyn_cast<MemberRefExpr>(e)) {
29432940
// If our m is a pure noncopyable type or our base is, we need to perform
29442941
// a noncopyable base borrow.
@@ -2949,8 +2946,39 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
29492946
return m->getType()->isNoncopyable() ||
29502947
m->getBase()->getType()->isNoncopyable();
29512948
}
2952-
if (auto *d = dyn_cast<DeclRefExpr>(e))
2953-
return e->getType()->isNoncopyable();
2949+
2950+
if (auto *le = dyn_cast<LoadExpr>(e)) {
2951+
// Noncopyable type is obviously noncopyable.
2952+
if (le->getType()->isNoncopyable()) {
2953+
return true;
2954+
}
2955+
// Otherwise, check if the thing we're loading from is a noncopyable
2956+
// param decl.
2957+
e = le->getSubExpr();
2958+
// fall through...
2959+
}
2960+
2961+
if (auto *de = dyn_cast<DeclRefExpr>(e)) {
2962+
// Noncopyable type is obviously noncopyable.
2963+
if (de->getType()->isNoncopyable()) {
2964+
return true;
2965+
}
2966+
// If the decl ref refers to a parameter with an explicit ownership
2967+
// modifier, it is not implicitly copyable.
2968+
if (auto pd = dyn_cast<ParamDecl>(de->getDecl())) {
2969+
switch (pd->getSpecifier()) {
2970+
case ParamSpecifier::Borrowing:
2971+
case ParamSpecifier::Consuming:
2972+
return true;
2973+
case ParamSpecifier::Default:
2974+
case ParamSpecifier::InOut:
2975+
case ParamSpecifier::LegacyShared:
2976+
case ParamSpecifier::LegacyOwned:
2977+
return false;
2978+
}
2979+
llvm_unreachable("unhandled switch case");
2980+
}
2981+
}
29542982
return false;
29552983
}
29562984

‎lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp‎

Lines changed: 1 addition & 219 deletions
Original file line numberDiff line numberDiff line change
@@ -82,225 +82,7 @@ bool swift::siloptimizer::
8282
if (!mmci || !mmci->hasMoveCheckerKind() || !mmci->getType().isObject())
8383
continue;
8484

85-
// Handle guaranteed/owned move arguments and values.
86-
//
87-
// We are pattern matching against these patterns:
88-
//
89-
// bb0(%0 : @guaranteed $T):
90-
// %1 = copy_value %0
91-
// %2 = mark_unresolved_non_copyable_value [no_consume_or_assign] %1
92-
// bb0(%0 : @owned $T):
93-
// %1 = mark_unresolved_non_copyable_value [no_consume_or_assign] %2
94-
//
95-
// This is forming a let or an argument.
96-
// bb0:
97-
// %1 = move_value [lexical] %0
98-
// %2 = mark_unresolved_non_copyable_value [consumable_and_assignable]
99-
// %1
100-
//
101-
// This occurs when SILGen materializes a temporary move only value?
102-
// bb0:
103-
// %1 = begin_borrow [lexical] %0
104-
// %2 = copy_value %1
105-
// %3 = mark_unresolved_non_copyable_value [no_consume_or_assign] %2
106-
if (mmci->getOperand()->getType().isMoveOnly() &&
107-
!mmci->getOperand()->getType().isMoveOnlyWrapped()) {
108-
if (auto *cvi = dyn_cast<CopyValueInst>(mmci->getOperand())) {
109-
if (auto *arg = dyn_cast<SILFunctionArgument>(cvi->getOperand())) {
110-
if (arg->getOwnershipKind() == OwnershipKind::Guaranteed) {
111-
moveIntroducersToProcess.insert(mmci);
112-
continue;
113-
}
114-
}
115-
116-
// In the case we have a resilient argument, we may have the following
117-
// pattern:
118-
//
119-
// bb0(%0 : $*Type): // in_guaranteed
120-
// %1 = load_borrow %0
121-
// %2 = copy_value
122-
// %3 = mark_unresolved_non_copyable_value [no_copy_or_assign]
123-
if (auto *lbi = dyn_cast<LoadBorrowInst>(cvi->getOperand())) {
124-
if (auto *arg = dyn_cast<SILFunctionArgument>(lbi->getOperand())) {
125-
if (arg->getKnownParameterInfo().isIndirectInGuaranteed()) {
126-
moveIntroducersToProcess.insert(mmci);
127-
continue;
128-
}
129-
}
130-
}
131-
132-
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
133-
if (bbi->isLexical()) {
134-
moveIntroducersToProcess.insert(mmci);
135-
continue;
136-
}
137-
}
138-
}
139-
140-
// Any time we have a move_value, we can process it.
141-
if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
142-
moveIntroducersToProcess.insert(mmci);
143-
continue;
144-
}
145-
146-
if (auto *arg = dyn_cast<SILFunctionArgument>(mmci->getOperand())) {
147-
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
148-
moveIntroducersToProcess.insert(mmci);
149-
continue;
150-
}
151-
}
152-
}
153-
154-
// Handle guaranteed arguments.
155-
//
156-
// We are pattern matching this pattern:
157-
//
158-
// bb0(%0 : @guaranteed $T):
159-
// %1 = copyable_to_moveonlywrapper [guaranteed] %0
160-
// %2 = copy_value %1
161-
// %3 = mark_unresolved_non_copyable_value [no_consume_or_assign] %2
162-
//
163-
// NOTE: Unlike with owned arguments, we do not need to insert a
164-
// begin_borrow lexical since the lexical value comes from the guaranteed
165-
// argument itself.
166-
//
167-
// NOTE: When we are done checking, we will eliminate the copy_value,
168-
// mark_unresolved_non_copyable_value inst to leave the IR in a guaranteed
169-
// state.
170-
if (auto *cvi = dyn_cast<CopyValueInst>(mmci->getOperand())) {
171-
if (auto *cvt = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
172-
cvi->getOperand())) {
173-
if (auto *arg = dyn_cast<SILFunctionArgument>(cvt->getOperand())) {
174-
if (arg->isNoImplicitCopy() &&
175-
arg->getOwnershipKind() == OwnershipKind::Guaranteed) {
176-
moveIntroducersToProcess.insert(mmci);
177-
continue;
178-
}
179-
}
180-
}
181-
}
182-
183-
// Handle trivial arguments and values.
184-
//
185-
// In the instruction stream this looks like:
186-
//
187-
// bb0(%0 : $Trivial):
188-
// %1 = copyable_to_moveonlywrapper [owned] %0
189-
// %2 = move_value [lexical] %1
190-
// %3 = mark_unresolved_non_copyable_value [consumable_and_assignable] %2
191-
//
192-
// *OR*
193-
//
194-
// bb0(%0 : $Trivial):
195-
// %1 = copyable_to_moveonlywrapper [owned] %0
196-
// %2 = move_value [lexical] %1
197-
// %3 = mark_unresolved_non_copyable_value [no_consume_or_assign] %2
198-
//
199-
// We are relying on a structural SIL requirement that %0 has only one
200-
// use, %1. This is validated by the SIL verifier. In this case, we need
201-
// the move_value [lexical] to ensure that we get a lexical scope for the
202-
// non-trivial value.
203-
if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
204-
if (mvi->isLexical()) {
205-
if (auto *cvt = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
206-
mvi->getOperand())) {
207-
if (cvt->getOperand()->getType().isTrivial(*fn)) {
208-
moveIntroducersToProcess.insert(mmci);
209-
continue;
210-
}
211-
}
212-
}
213-
}
214-
215-
// Handle owned arguments.
216-
//
217-
// We are pattern matching this:
218-
//
219-
// bb0(%0 : @owned $T):
220-
// %1 = copyable_to_moveonlywrapper [owned] %0
221-
// %2 = move_value [lexical] %1
222-
// %3 = mark_unresolved_non_copyable_value
223-
// [consumable_and_assignable_owned] %2
224-
if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
225-
if (mvi->isLexical()) {
226-
if (auto *cvt = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
227-
mvi->getOperand())) {
228-
if (auto *arg = dyn_cast<SILFunctionArgument>(cvt->getOperand())) {
229-
if (arg->isNoImplicitCopy()) {
230-
moveIntroducersToProcess.insert(mmci);
231-
continue;
232-
}
233-
}
234-
}
235-
}
236-
}
237-
238-
// Handle non-trivial values.
239-
//
240-
// We are looking for the following pattern:
241-
//
242-
// %1 = begin_borrow [lexical] %0
243-
// %2 = copy_value %1
244-
// %3 = copyable_to_moveonlywrapper [owned] %2
245-
// %4 = mark_unresolved_non_copyable_value [consumable_and_assignable]
246-
//
247-
// Or for a move only type, we look for a move_value [lexical].
248-
if (auto *mvi = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
249-
mmci->getOperand())) {
250-
if (auto *cvi = dyn_cast<CopyValueInst>(mvi->getOperand())) {
251-
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
252-
if (bbi->isLexical()) {
253-
moveIntroducersToProcess.insert(mmci);
254-
continue;
255-
}
256-
}
257-
}
258-
}
259-
260-
// Handle trivial values.
261-
//
262-
// We pattern match:
263-
//
264-
// %1 = copyable_to_moveonlywrapper [owned] %0
265-
// %2 = move_value [lexical] %1
266-
// %3 = mark_unresolved_non_copyable_value [consumable_and_assignable] %2
267-
if (auto *cvi = dyn_cast<ExplicitCopyValueInst>(mmci->getOperand())) {
268-
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
269-
if (bbi->isLexical()) {
270-
moveIntroducersToProcess.insert(mmci);
271-
continue;
272-
}
273-
}
274-
}
275-
276-
// Handle guaranteed parameters of a resilient type used by a resilient
277-
// function inside the module in which the resilient type is defined.
278-
if (auto *cvi = dyn_cast<CopyValueInst>(mmci->getOperand())) {
279-
if (auto *cmi = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(cvi->getOperand())) {
280-
if (auto *lbi = dyn_cast<LoadBorrowInst>(cmi->getOperand())) {
281-
if (auto *arg = dyn_cast<SILFunctionArgument>(lbi->getOperand())) {
282-
if (arg->getKnownParameterInfo().isIndirectInGuaranteed()) {
283-
moveIntroducersToProcess.insert(mmci);
284-
continue;
285-
}
286-
}
287-
}
288-
}
289-
}
290-
291-
// If we see a mark_unresolved_non_copyable_value that is marked no
292-
// implicit copy that we don't understand, emit a diagnostic to fail the
293-
// compilation. This ensures that if someone marks something no implicit
294-
// copy and we fail to check it, we fail the compilation.
295-
//
296-
// We then RAUW the mark_unresolved_non_copyable_value once we have
297-
// emitted the error since later passes expect that
298-
// mark_unresolved_non_copyable_value has been eliminated by us. Since we
299-
// are failing already, this is ok to do.
300-
emitter.emitCheckerDoesntUnderstandDiagnostic(mmci);
301-
mmci->replaceAllUsesWith(mmci->getOperand());
302-
mmci->eraseFromParent();
303-
localChanged = true;
85+
moveIntroducersToProcess.insert(mmci);
30486
}
30587
}
30688
return localChanged;

‎test/SILGen/cf_members.swift‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,8 @@ public func foo(_ x: Double) {
124124
// CHECK: [[READ:%.*]] = begin_access [read] [unknown] [[Z]] : $*Struct1
125125
// CHECK: [[ZVAL:%.*]] = load [trivial] [[READ]]
126126
// CHECK: store [[ZVAL]] to [trivial] [[ZTMP:%.*]] :
127-
// CHECK: [[ZVAL_2:%.*]] = load [trivial] [[ZTMP]]
128-
// CHECK: store [[ZVAL_2]] to [trivial] [[ZTMP_2:%.*]] :
129127
// CHECK: [[GET:%.*]] = function_ref @IAMStruct1GetRadius : $@convention(c) (@in Struct1) -> Double
130-
// CHECK: apply [[GET]]([[ZTMP_2]])
128+
// CHECK: apply [[GET]]([[ZTMP]])
131129
_ = z.radius
132130
// CHECK: [[READ:%.*]] = begin_access [read] [unknown] [[Z]] : $*Struct1
133131
// CHECK: [[ZVAL:%.*]] = load [trivial] [[READ]]

‎test/SILOptimizer/moveonly_loadable_to_address_reabstraction.swift‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// RUN: %target-swift-frontend -emit-sil -verify -enable-library-evolution -enable-experimental-feature MoveOnlyResilientTypes %s
2+
3+
// Verify that call sequences that require reabstracting a noncopyable value
4+
// from a loadable representation to an in-memory one are properly allowed by
5+
// the move-only checker.
6+
7+
public struct Foo: ~Copyable {
8+
private var x: String = ""
9+
10+
public var isEmpty: Bool {
11+
@_silgen_name("get_isEmpty") get
12+
}
13+
}
14+
15+
public struct Bar: ~Copyable {
16+
public var x = Foo()
17+
18+
public consuming func foo() {
19+
// `Foo` is internally a loadable type, but it's public with library
20+
// evolution enabled, so public members like `isEmpty` take their
21+
// argument indirectly.
22+
if x.isEmpty {}
23+
//else { bar(x) }
24+
}
25+
}
26+
27+
@_silgen_name("bar")
28+
func bar(_: consuming Foo)
29+
30+
func foo(_ x: consuming Foo) {
31+
// `[String]` is a loadable copyable type, but we're using the
32+
// `consuming` modifier on `x` to suppress implicit copyability.
33+
// `isEmpty` is a method from the Collection protocol, so it takes `self`
34+
// generically and therefore indirectly.
35+
if x.isEmpty {}
36+
else { bar(x) }
37+
}
38+
39+
func copyableBar(_: consuming [String]) {}
40+
41+
func copyableFoo(prefix: consuming [String]) {
42+
if prefix.isEmpty { }
43+
else { copyableBar(prefix) }
44+
}
45+
46+
struct CopyableFoo {
47+
var prefix: [String]
48+
49+
consuming func foo() {
50+
if prefix.isEmpty { }
51+
else { copyableBar(prefix) }
52+
}
53+
}

0 commit comments

Comments
 (0)