Skip to content

Commit d54555b

Browse files
gottesmmgottesmm
authored
Merge pull request #62046 from gottesmm/pr-612f7d0a758fc151826d72241af5276095ef3409
[move-only] When passing a move only struct address, pass it as a true borrowed parameter.
2 parents 9bdf395 + 525ba8c commit d54555b

File tree

3 files changed

+202
-5
lines changed

3 files changed

+202
-5
lines changed

‎lib/SILGen/ArgumentSource.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ class ArgumentSource {
223223
}
224224

225225
Expr *findStorageReferenceExprForBorrow() &&;
226+
Expr *findStorageReferenceExprForMoveOnlyBorrow() &&;
226227

227228
/// Given that this source is an expression, extract and clear
228229
/// that expression.

‎lib/SILGen/SILGenApply.cpp

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,27 @@
2525
#include "Varargs.h"
2626
#include "swift/AST/ASTContext.h"
2727
#include "swift/AST/DiagnosticsSIL.h"
28+
#include "swift/AST/DistributedDecl.h"
2829
#include "swift/AST/Effects.h"
30+
#include "swift/AST/Expr.h"
2931
#include "swift/AST/ForeignAsyncConvention.h"
3032
#include "swift/AST/ForeignErrorConvention.h"
3133
#include "swift/AST/GenericEnvironment.h"
32-
#include "swift/AST/DistributedDecl.h"
3334
#include "swift/AST/GenericSignature.h"
3435
#include "swift/AST/Module.h"
3536
#include "swift/AST/ModuleLoader.h"
3637
#include "swift/AST/ParameterList.h"
3738
#include "swift/AST/SubstitutionMap.h"
3839
#include "swift/Basic/ExternalUnion.h"
3940
#include "swift/Basic/Range.h"
40-
#include "swift/Basic/SourceManager.h"
4141
#include "swift/Basic/STLExtras.h"
42+
#include "swift/Basic/SourceManager.h"
4243
#include "swift/Basic/Unicode.h"
4344
#include "swift/SIL/PrettyStackTrace.h"
4445
#include "swift/SIL/SILArgument.h"
4546
#include "clang/AST/DeclCXX.h"
46-
#include "llvm/Support/Compiler.h"
4747
#include "clang/AST/DeclObjC.h"
48+
#include "llvm/Support/Compiler.h"
4849

4950
using namespace swift;
5051
using namespace Lowering;
@@ -2921,6 +2922,25 @@ static Expr *findStorageReferenceExprForBorrow(Expr *e) {
29212922
return nullptr;
29222923
}
29232924

2925+
Expr *ArgumentSource::findStorageReferenceExprForMoveOnlyBorrow() && {
2926+
if (!isExpr())
2927+
return nullptr;
2928+
2929+
auto argExpr = asKnownExpr();
2930+
auto *li = dyn_cast<LoadExpr>(argExpr);
2931+
if (!li)
2932+
return nullptr;
2933+
2934+
auto lvExpr = ::findStorageReferenceExprForBorrow(li->getSubExpr());
2935+
2936+
// Claim the value of this argument if we found a storage reference.
2937+
if (lvExpr) {
2938+
(void)std::move(*this).asKnownExpr();
2939+
}
2940+
2941+
return lvExpr;
2942+
}
2943+
29242944
Expr *ArgumentSource::findStorageReferenceExprForBorrow() && {
29252945
if (!isExpr()) return nullptr;
29262946

@@ -3057,6 +3077,13 @@ class ArgEmitter {
30573077
return;
30583078
}
30593079

3080+
if (loweredSubstArgType.isPureMoveOnly() && param.isGuaranteed()) {
3081+
if (tryEmitBorrowedMoveOnly(std::move(arg), loweredSubstArgType,
3082+
loweredSubstParamType, origParamType,
3083+
paramSlice))
3084+
return;
3085+
}
3086+
30603087
if (SGF.silConv.isSILIndirect(param)) {
30613088
emitIndirect(std::move(arg), loweredSubstArgType, origParamType, param);
30623089
return;
@@ -3226,6 +3253,23 @@ class ArgEmitter {
32263253
return true;
32273254
}
32283255

3256+
bool tryEmitBorrowedMoveOnly(ArgumentSource &&arg,
3257+
SILType loweredSubstArgType,
3258+
SILType loweredSubstParamType,
3259+
AbstractionPattern origParamType,
3260+
ClaimedParamsRef paramsSlice) {
3261+
assert(paramsSlice.size() == 1);
3262+
3263+
// Try to find an expression we can emit as an l-value.
3264+
auto lvExpr = std::move(arg).findStorageReferenceExprForMoveOnlyBorrow();
3265+
if (!lvExpr)
3266+
return false;
3267+
3268+
emitBorrowed(lvExpr, loweredSubstArgType, loweredSubstParamType,
3269+
origParamType, paramsSlice);
3270+
return true;
3271+
}
3272+
32293273
void emitBorrowed(Expr *arg, SILType loweredSubstArgType,
32303274
SILType loweredSubstParamType,
32313275
AbstractionPattern origParamType,

‎test/SILGen/moveonly.swift

Lines changed: 154 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,39 @@
44
// Declarations //
55
//////////////////
66

7+
@_moveOnly
8+
public class Klass2 {}
9+
710
@_moveOnly
811
public class Klass {
912
var intField: Int
10-
var klsField: Klass?
13+
var klsField: Klass2
1114

1215
// CHECK-LABEL: sil hidden [ossa] @$s8moveonly5KlassCACycfc : $@convention(method) (@owned Klass) -> @owned Klass {
1316
// CHECK: bb0([[ARG:%.*]] : @owned $Klass):
1417
// CHECK: [[MARK_UNINIT:%.*]] = mark_uninitialized [rootself] [[ARG]]
1518
// CHECK: [[MARK_NO_IMP_COPY:%.*]] = mark_must_check [no_implicit_copy] [[MARK_UNINIT]]
1619
// CHECK: } // end sil function '$s8moveonly5KlassCACycfc'
1720
init() {
18-
klsField = Klass()
21+
klsField = Klass2()
1922
intField = 5
2023
}
2124
}
2225

2326
public func nonConsumingUseKlass(_ k: Klass) {}
27+
public func nonConsumingUseKlass2(_ k: Klass2) {}
28+
29+
@_moveOnly
30+
public struct NonTrivialStruct2 {
31+
var k = Klass()
32+
}
33+
34+
public func nonConsumingUseNonTrivialStruct2(_ s: NonTrivialStruct2) {}
2435

2536
@_moveOnly
2637
public struct NonTrivialStruct {
2738
var k = Klass()
39+
var k2 = NonTrivialStruct2()
2840
}
2941

3042
public func nonConsumingUseNonTrivialStruct(_ s: NonTrivialStruct) {}
@@ -243,3 +255,143 @@ func blackHoleVarInitialization2() {
243255
x = Klass()
244256
var _ = x
245257
}
258+
259+
////////////////////////////////
260+
// Borrow Function Call Tests //
261+
////////////////////////////////
262+
263+
// CHECK-LABEL: sil hidden [ossa] @$s8moveonly24borrowObjectFunctionCallyyF : $@convention(thin) () -> () {
264+
// CHECK: [[CLS:%.*]] = mark_must_check [no_implicit_copy]
265+
// CHECK: [[BORROW:%.*]] = begin_borrow [[CLS]]
266+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly20nonConsumingUseKlassyyAA0E0CF
267+
// CHECK: apply [[FN]]([[BORROW]])
268+
// CHECK: end_borrow [[BORROW]]
269+
// CHECK: } // end sil function '$s8moveonly24borrowObjectFunctionCallyyF'
270+
func borrowObjectFunctionCall() {
271+
let k = Klass()
272+
nonConsumingUseKlass(k)
273+
}
274+
275+
// CHECK-LABEL: sil hidden [ossa] @$s8moveonly25borrowAddressFunctionCallyyF : $@convention(thin) () -> () {
276+
// CHECK: [[BOX:%.*]] = mark_must_check [no_implicit_copy]
277+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[BOX]]
278+
// CHECK: [[BORROW:%.*]] = load_borrow [[ACCESS]]
279+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly20nonConsumingUseKlassyyAA0E0CF
280+
// CHECK: apply [[FN]]([[BORROW]])
281+
// CHECK: end_borrow [[BORROW]]
282+
// CHECK: end_access [[ACCESS]]
283+
// CHECK: } // end sil function '$s8moveonly25borrowAddressFunctionCallyyF'
284+
func borrowAddressFunctionCall() {
285+
var k = Klass()
286+
k = Klass()
287+
nonConsumingUseKlass(k)
288+
}
289+
290+
// We currently have the wrong behavior here since the class is treated by
291+
// LValue emission as a base. That being said, move only classes are not our
292+
// high order bit here, so I filed: ...;
293+
//
294+
// CHECK-LABEL: sil hidden [ossa] @$s8moveonly31klassBorrowAddressFunctionCall2yyF : $@convention(thin) () -> () {
295+
// CHECK: [[MARK:%.*]] = mark_must_check [no_implicit_copy]
296+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARK]]
297+
// CHECK: } // end sil function '$s8moveonly31klassBorrowAddressFunctionCall2yyF'
298+
func klassBorrowAddressFunctionCall2() {
299+
var k = Klass()
300+
k = Klass()
301+
nonConsumingUseKlass2(k.klsField)
302+
}
303+
304+
// CHECK-LABEL: sil hidden [ossa] @$s8moveonly31structBorrowObjectFunctionCall1yyF : $@convention(thin) () -> () {
305+
// CHECK: [[VALUE:%.*]] = mark_must_check [no_implicit_copy]
306+
// CHECK: [[BORROW:%.*]] = begin_borrow [[VALUE]]
307+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly31nonConsumingUseNonTrivialStructyyAA0efG0VF : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
308+
// CHECK: apply [[FN]]([[BORROW]])
309+
// CHECK: end_borrow [[BORROW]]
310+
// CHECK: [[BORROW:%.*]] = begin_borrow [[VALUE]]
311+
// CHECK: [[STRUCT_EXT:%.*]] = struct_extract [[BORROW]]
312+
// CHECK: [[STRUCT_EXT_COPY:%.*]] = copy_value [[STRUCT_EXT]]
313+
// TODO: Should we insert a mark_must_check_here?
314+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly20nonConsumingUseKlassyyAA0E0CF
315+
// CHECK: apply [[FN]]([[STRUCT_EXT_COPY]])
316+
// CHECK: destroy_value [[STRUCT_EXT_COPY]]
317+
// CHECK: end_borrow [[BORROW]]
318+
// CHECK: [[BORROW:%.*]] = begin_borrow [[VALUE]]
319+
// CHECK: [[STRUCT_EXT:%.*]] = struct_extract [[BORROW]]
320+
// CHECK: [[STRUCT_EXT_COPY:%.*]] = copy_value [[STRUCT_EXT]]
321+
// CHECK: [[STRUCT_EXT_COPY_BORROW:%.*]] = begin_borrow [[STRUCT_EXT_COPY]]
322+
// CHECK: [[METHOD:%.*]] = class_method [[STRUCT_EXT_COPY_BORROW]]
323+
// CHECK: [[KLS2:%.*]] = apply [[METHOD]]([[STRUCT_EXT_COPY_BORROW]])
324+
// CHECK: end_borrow [[STRUCT_EXT_COPY_BORROW]]
325+
// CHECK: [[BORROWED_KLS2:%.*]] = begin_borrow [[KLS2]]
326+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly21nonConsumingUseKlass2yyAA0E0CF
327+
// CHECK: apply [[FN]]([[BORROWED_KLS2]])
328+
// CHECK: destroy_value [[STRUCT_EXT_COPY]]
329+
// CHECK: destroy_value [[KLS2]]
330+
// CHECK: end_borrow [[BORROW]]
331+
// CHECK: destroy_value [[VALUE]]
332+
// CHECK: } // end sil function '$s8moveonly31structBorrowObjectFunctionCall1yyF'
333+
func structBorrowObjectFunctionCall1() {
334+
let k = NonTrivialStruct()
335+
nonConsumingUseNonTrivialStruct(k)
336+
nonConsumingUseKlass(k.k)
337+
nonConsumingUseKlass2(k.k.klsField)
338+
}
339+
340+
// CHECK-LABEL: sil hidden [ossa] @$s8moveonly31structBorrowObjectFunctionCall2yyF : $@convention(thin) () -> () {
341+
// CHECK: [[MARKED_ADDR:%.*]] = mark_must_check [no_implicit_copy]
342+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARKED_ADDR]]
343+
// CHECK: [[BORROW:%.*]] = load_borrow [[ACCESS]]
344+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly31nonConsumingUseNonTrivialStructyyAA0efG0VF :
345+
// CHECK: apply [[FN]]([[BORROW]])
346+
// CHECK: end_borrow [[BORROW]]
347+
// CHECK: end_access [[ACCESS]]
348+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARKED_ADDR]]
349+
// CHECK: [[STRUCT_EXT:%.*]] = struct_element_addr [[ACCESS]]
350+
// CHECK: [[BORROW:%.*]] = load_borrow [[STRUCT_EXT]]
351+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly20nonConsumingUseKlassyyAA0E0CF
352+
// CHECK: apply [[FN]]([[BORROW]])
353+
// CHECK: end_borrow [[BORROW]]
354+
// CHECK: end_access [[ACCESS]]
355+
//
356+
// This case we fail due to the class being a base element. We need to use the
357+
// scope expansion approach to handle this.
358+
//
359+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARKED_ADDR]]
360+
// CHECK: [[STRUCT_EXT:%.*]] = struct_element_addr [[ACCESS]]
361+
// CHECK: [[STRUCT_EXT_COPY:%.*]] = load [copy] [[STRUCT_EXT]]
362+
// CHECK: end_access [[ACCESS]]
363+
// CHECK: [[STRUCT_EXT_COPY_BORROW:%.*]] = begin_borrow [[STRUCT_EXT_COPY]]
364+
// CHECK: [[METHOD:%.*]] = class_method [[STRUCT_EXT_COPY_BORROW]]
365+
// CHECK: [[KLS:%.*]] = apply [[METHOD]]([[STRUCT_EXT_COPY_BORROW]])
366+
// CHECK: end_borrow [[STRUCT_EXT_COPY_BORROW]]
367+
// CHECK: [[BORROWED_KLS:%.*]] = begin_borrow [[KLS]]
368+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly21nonConsumingUseKlass2yyAA0E0CF
369+
// CHECK: apply [[FN]]([[BORROWED_KLS]])
370+
// CHECK: end_borrow [[BORROWED_KLS]]
371+
// CHECK: destroy_value [[STRUCT_EXT_COPY]]
372+
// CHECK: destroy_value [[KLS]]
373+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARKED_ADDR]]
374+
// CHECK: [[GEP:%.*]] = struct_element_addr [[ACCESS]] : $*NonTrivialStruct, #NonTrivialStruct.k2
375+
// CHECK: [[BORROW:%.*]] = load_borrow [[GEP]]
376+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly32nonConsumingUseNonTrivialStruct2yyAA0efG0VF : $@convention(thin) (@guaranteed NonTrivialStruct2) -> ()
377+
// CHECK: apply [[FN]]([[BORROW]])
378+
// CHECK: end_borrow [[BORROW]]
379+
// CHECK: end_access [[ACCESS]]
380+
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARKED_ADDR]]
381+
// CHECK: [[GEP1:%.*]] = struct_element_addr [[ACCESS]] : $*NonTrivialStruct, #NonTrivialStruct.k2
382+
// CHECK: [[GEP2:%.*]] = struct_element_addr [[GEP1]] : $*NonTrivialStruct2, #NonTrivialStruct2.k
383+
// CHECK: [[BORROW:%.*]] = load_borrow [[GEP2]]
384+
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly20nonConsumingUseKlassyyAA0E0CF : $@convention(thin) (@guaranteed Klass) -> ()
385+
// CHECK: apply [[FN]]([[BORROW]])
386+
// CHECK: end_borrow [[BORROW]]
387+
// CHECK: end_access [[ACCESS]]
388+
// CHECK: } // end sil function '$s8moveonly31structBorrowObjectFunctionCall2yyF'
389+
func structBorrowObjectFunctionCall2() {
390+
var k = NonTrivialStruct()
391+
k = NonTrivialStruct()
392+
nonConsumingUseNonTrivialStruct(k)
393+
nonConsumingUseKlass(k.k)
394+
nonConsumingUseKlass2(k.k.klsField)
395+
nonConsumingUseNonTrivialStruct2(k.k2)
396+
nonConsumingUseKlass(k.k2.k)
397+
}

0 commit comments

Comments
 (0)