Skip to content

Commit 66df502

Browse files
committed
[move-only] In the address checker, make sure that load [copy] of copyable fields are not considered loads that need to be consumed.
Just something I have been meaning to fix for a little bit. rdar://104753490
1 parent 8738c81 commit 66df502

File tree

3 files changed

+57
-10
lines changed

3 files changed

+57
-10
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,18 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
10961096
// emits code of this form and we need to recognize it as a copy of the
10971097
// underlying var.
10981098
if (auto *li = dyn_cast<LoadInst>(user)) {
1099+
// Before we do anything, see if this load is of a copyable field or is a
1100+
// trivial load. If it is, then we just treat this as a liveness requiring
1101+
// use.
1102+
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Trivial ||
1103+
!li->getType().isMoveOnly()) {
1104+
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
1105+
if (!leafRange)
1106+
return false;
1107+
useState.livenessUses.insert({user, *leafRange});
1108+
return true;
1109+
}
1110+
10991111
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Copy ||
11001112
li->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
11011113
SWIFT_DEFER { moveChecker.canonicalizer.clear(); };

test/SILOptimizer/moveonly_addresschecker_diagnostics.sil

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ bb3:
140140
sil hidden [ossa] @copyableKlassInAMoveOnlyStruct2 : $@convention(thin) (@owned NonTrivialStruct, @owned NonTrivialStruct) -> () {
141141
bb0(%arg : @owned $NonTrivialStruct, %arg1 : @owned $NonTrivialStruct):
142142
%0 = alloc_stack [lexical] $NonTrivialStruct, var, name "a"
143-
%1 = mark_must_check [no_implicit_copy] %0 : $*NonTrivialStruct // expected-error {{'a' consumed more than once}}
143+
%1 = mark_must_check [no_implicit_copy] %0 : $*NonTrivialStruct
144144
store %arg to [init] %1 : $*NonTrivialStruct
145145
%9 = begin_access [modify] [static] %1 : $*NonTrivialStruct
146146
store %arg1 to [assign] %9 : $*NonTrivialStruct
@@ -154,18 +154,51 @@ bb0(%arg : @owned $NonTrivialStruct, %arg1 : @owned $NonTrivialStruct):
154154
end_access %12 : $*NonTrivialStruct
155155
%19 = begin_access [read] [static] %1 : $*NonTrivialStruct
156156
%20 = struct_element_addr %19 : $*NonTrivialStruct, #NonTrivialStruct.copyableK
157-
%21 = load [copy] %20 : $*CopyableKlass // expected-note {{consuming use}}
157+
%21 = load [copy] %20 : $*CopyableKlass
158158
end_access %19 : $*NonTrivialStruct
159159
%23 = function_ref @copyableClassConsume : $@convention(thin) (@owned CopyableKlass) -> ()
160160
%24 = apply %23(%21) : $@convention(thin) (@owned CopyableKlass) -> ()
161161
%25 = begin_access [read] [static] %1 : $*NonTrivialStruct
162162
%26 = struct_element_addr %25 : $*NonTrivialStruct, #NonTrivialStruct.copyableK
163-
%27 = load [copy] %26 : $*CopyableKlass // expected-note {{consuming use}}
163+
%27 = load [copy] %26 : $*CopyableKlass
164164
end_access %25 : $*NonTrivialStruct
165165
%29 = function_ref @copyableClassConsume : $@convention(thin) (@owned CopyableKlass) -> ()
166166
%30 = apply %29(%27) : $@convention(thin) (@owned CopyableKlass) -> ()
167167
destroy_addr %1 : $*NonTrivialStruct
168168
dealloc_stack %0 : $*NonTrivialStruct
169169
%33 = tuple ()
170170
return %33 : $()
171-
}
171+
}
172+
173+
sil [ossa] @moveOnlyKlassInAMoveOnlyStruct2 : $@convention(thin) (@owned NonTrivialStruct, @owned NonTrivialStruct) -> () {
174+
bb0(%arg : @owned $NonTrivialStruct, %arg1 : @owned $NonTrivialStruct):
175+
%0 = alloc_stack [lexical] $NonTrivialStruct, var, name "a"
176+
%1 = mark_must_check [no_implicit_copy] %0 : $*NonTrivialStruct // expected-error {{'a' consumed more than once}}
177+
store %arg to [init] %1 : $*NonTrivialStruct
178+
%9 = begin_access [modify] [static] %1 : $*NonTrivialStruct
179+
store %arg1 to [assign] %9 : $*NonTrivialStruct
180+
end_access %9 : $*NonTrivialStruct
181+
%12 = begin_access [read] [static] %1 : $*NonTrivialStruct
182+
%13 = struct_element_addr %12 : $*NonTrivialStruct, #NonTrivialStruct.k
183+
%14 = load_borrow %13 : $*Klass
184+
%15 = function_ref @nonConsumingUseKlass : $@convention(thin) (@guaranteed Klass) -> ()
185+
%16 = apply %15(%14) : $@convention(thin) (@guaranteed Klass) -> ()
186+
end_borrow %14 : $Klass
187+
end_access %12 : $*NonTrivialStruct
188+
%19 = begin_access [read] [static] %1 : $*NonTrivialStruct
189+
%20 = struct_element_addr %19 : $*NonTrivialStruct, #NonTrivialStruct.k
190+
%21 = load [copy] %20 : $*Klass // expected-note {{consuming use}}
191+
end_access %19 : $*NonTrivialStruct
192+
%23 = function_ref @classConsume : $@convention(thin) (@owned Klass) -> ()
193+
%24 = apply %23(%21) : $@convention(thin) (@owned Klass) -> ()
194+
%25 = begin_access [read] [static] %1 : $*NonTrivialStruct
195+
%26 = struct_element_addr %25 : $*NonTrivialStruct, #NonTrivialStruct.k
196+
%27 = load [copy] %26 : $*Klass // expected-note {{consuming use}}
197+
end_access %25 : $*NonTrivialStruct
198+
%29 = function_ref @classConsume : $@convention(thin) (@owned Klass) -> ()
199+
%30 = apply %29(%27) : $@convention(thin) (@owned Klass) -> ()
200+
destroy_addr %1 : $*NonTrivialStruct
201+
dealloc_stack %0 : $*NonTrivialStruct
202+
%33 = tuple ()
203+
return %33 : $()
204+
}

test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2259,20 +2259,22 @@ func copyableKlassInAMoveOnlyStruct() {
22592259
copyableClassConsume(a.copyableK)
22602260
}
22612261

2262+
// This shouldn't error since we are consuming a copyable type.
22622263
func copyableKlassInAMoveOnlyStruct2() {
2263-
var a = NonTrivialStruct() // expected-error {{'a' consumed more than once}}
2264+
var a = NonTrivialStruct()
22642265
a = NonTrivialStruct()
22652266
copyableClassUseMoveOnlyWithoutEscaping(a.copyableK)
2266-
copyableClassConsume(a.copyableK) // expected-note {{consuming use}}
2267-
copyableClassConsume(a.copyableK) // expected-note {{consuming use}}
2267+
copyableClassConsume(a.copyableK)
2268+
copyableClassConsume(a.copyableK)
22682269
}
22692270

2271+
// This shouldn't error since we are working with a copyable type.
22702272
func copyableKlassInAMoveOnlyStruct3() {
2271-
var a = NonTrivialStruct() // expected-error {{'a' used after consume. Lifetime extension of variable requires a copy}}
2273+
var a = NonTrivialStruct()
22722274
a = NonTrivialStruct()
22732275
copyableClassUseMoveOnlyWithoutEscaping(a.copyableK)
2274-
copyableClassConsume(a.copyableK) // expected-note {{consuming use}}
2275-
copyableClassUseMoveOnlyWithoutEscaping(a.copyableK) // expected-note {{non-consuming use}}
2276+
copyableClassConsume(a.copyableK)
2277+
copyableClassUseMoveOnlyWithoutEscaping(a.copyableK)
22762278
}
22772279

22782280
// This used to error, but no longer errors since we are using a true field

0 commit comments

Comments
 (0)