Skip to content

Commit 8f9772d

Browse files
gottesmmgottesmm
committed
[move-only] Make sure to treat (mark_must_check (begin_access (project_box (@capture box arg)))) like an inout term user requiring situation.
We already did this for the situation without the begin_access. In truth, using the terminator is a bit too wide, but it works for these sorts of arguments that use assignable_but_not_consumable so for expediency (and since we are just walking blocks), I just decided to do something quick. rdar://106208343
1 parent 8862b1b commit 8f9772d

File tree

3 files changed

+119
-25
lines changed

3 files changed

+119
-25
lines changed

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -338,33 +338,50 @@ static bool memInstMustConsume(Operand *memOper) {
338338
/// These are cases where we want to treat the end of the function as a liveness
339339
/// use to ensure that we reinitialize \p value before the end of the function
340340
/// if we consume \p value in the function body.
341-
static bool isInOutDefThatNeedsEndOfFunctionLiveness(SILValue value) {
342-
if (auto *fArg = dyn_cast<SILFunctionArgument>(value)) {
343-
switch (fArg->getArgumentConvention()) {
344-
case SILArgumentConvention::Indirect_In:
345-
case SILArgumentConvention::Indirect_Out:
346-
case SILArgumentConvention::Indirect_In_Guaranteed:
347-
case SILArgumentConvention::Direct_Guaranteed:
348-
case SILArgumentConvention::Direct_Owned:
349-
case SILArgumentConvention::Direct_Unowned:
350-
case SILArgumentConvention::Pack_Guaranteed:
351-
case SILArgumentConvention::Pack_Owned:
352-
case SILArgumentConvention::Pack_Out:
353-
return false;
354-
case SILArgumentConvention::Indirect_Inout:
355-
case SILArgumentConvention::Indirect_InoutAliasable:
356-
case SILArgumentConvention::Pack_Inout:
357-
LLVM_DEBUG(llvm::dbgs() << "Found inout arg: " << *fArg);
358-
return true;
341+
static bool isInOutDefThatNeedsEndOfFunctionLiveness(MarkMustCheckInst *markedAddr) {
342+
SILValue operand = markedAddr->getOperand();
343+
344+
// Check for inout types of arguments that are marked with consumable and
345+
// assignable.
346+
if (markedAddr->getCheckKind() ==
347+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable) {
348+
if (auto *fArg = dyn_cast<SILFunctionArgument>(operand)) {
349+
switch (fArg->getArgumentConvention()) {
350+
case SILArgumentConvention::Indirect_In:
351+
case SILArgumentConvention::Indirect_Out:
352+
case SILArgumentConvention::Indirect_In_Guaranteed:
353+
case SILArgumentConvention::Direct_Guaranteed:
354+
case SILArgumentConvention::Direct_Owned:
355+
case SILArgumentConvention::Direct_Unowned:
356+
case SILArgumentConvention::Pack_Guaranteed:
357+
case SILArgumentConvention::Pack_Owned:
358+
case SILArgumentConvention::Pack_Out:
359+
return false;
360+
case SILArgumentConvention::Indirect_Inout:
361+
case SILArgumentConvention::Indirect_InoutAliasable:
362+
case SILArgumentConvention::Pack_Inout:
363+
LLVM_DEBUG(llvm::dbgs() << "Found inout arg: " << *fArg);
364+
return true;
365+
}
359366
}
360367
}
361368

362-
if (auto *pbi = dyn_cast<ProjectBoxInst>(value)) {
363-
if (auto *fArg = dyn_cast<SILFunctionArgument>(pbi->getOperand())) {
364-
if (!fArg->isClosureCapture())
365-
return false;
366-
LLVM_DEBUG(llvm::dbgs() << "Found inout arg: " << *fArg);
367-
return true;
369+
// See if we have an assignable_but_not_consumable from a project_box +
370+
// function_argument. In this case, the value must be live at the end of the
371+
// use, similar to an inout parameter.
372+
//
373+
// TODO: Rather than using a terminator, we might be able to use the
374+
// end_access of the access marker instead. That would slightly change the
375+
// model and this is semantically ok today.
376+
if (markedAddr->getCheckKind() ==
377+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable) {
378+
if (auto *pbi = dyn_cast<ProjectBoxInst>(stripAccessMarkers(operand))) {
379+
if (auto *fArg = dyn_cast<SILFunctionArgument>(pbi->getOperand())) {
380+
if (!fArg->isClosureCapture())
381+
return false;
382+
LLVM_DEBUG(llvm::dbgs() << "Found inout arg: " << *fArg);
383+
return true;
384+
}
368385
}
369386
}
370387

@@ -481,7 +498,7 @@ struct UseState {
481498
initializeLiveness(FieldSensitiveMultiDefPrunedLiveRange &prunedLiveness);
482499

483500
void initializeInOutTermUsers() {
484-
if (!isInOutDefThatNeedsEndOfFunctionLiveness(address->getOperand()))
501+
if (!isInOutDefThatNeedsEndOfFunctionLiveness(address))
485502
return;
486503

487504
SmallVector<SILBasicBlock *, 8> exitBlocks;

‎test/Interpreter/moveonly_escaping_capture_lifetimes.swift

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// RUN: %target-run-simple-swift(-Xfrontend -enable-experimental-move-only) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
5+
@_moveOnly
6+
struct MO {
7+
var x: Int
8+
deinit { print("dying \(x)") }
9+
}
10+
11+
var f: () -> () = {}
12+
13+
func foo(x: consuming MO) {
14+
var counter = 42
15+
f = {
16+
x = MO(x: counter)
17+
counter += 1
18+
}
19+
}
20+
21+
func main() {
22+
let x = MO(x: 69)
23+
24+
// CHECK: a
25+
print("a")
26+
foo(x: x)
27+
// CHECK-NEXT: b
28+
print("b")
29+
// CHECK-NEXT: dying 69
30+
// CHECK-NEXT: c
31+
f()
32+
print("c")
33+
// CHECK-NEXT: dying 42
34+
// CHECK-NEXT: d
35+
f()
36+
print("d")
37+
// CHECK-NEXT: dying 43
38+
// CHECK-NEXT: e
39+
f()
40+
print("e")
41+
// CHECK-NEXT: dying 44
42+
// CHECK-NEXT: f
43+
f = {}
44+
print("f")
45+
}
46+
main()
47+
print("done")

‎test/SILOptimizer/moveonly_addresschecker.sil

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public struct NonTrivialStruct {
2727
}
2828

2929
sil @useNonTrivialStruct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
30+
sil @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
3031

3132
@_moveOnly
3233
public struct NonTrivialStructPair {
@@ -437,3 +438,32 @@ bb0(%0 : $*Klass):
437438
%27 = tuple ()
438439
return %27 : $()
439440
}
441+
442+
// Make sure that we treat the move only access below like an inout parameter
443+
// that is live at the end of its lifetime, rather than like a var.
444+
//
445+
// CHECK: sil [ossa] @closure_assignable_but_not_consumable_treated_like_inout : $@convention(thin) (@guaranteed { var NonTrivialStruct }) -> () {
446+
// CHECK: bb0([[ARG:%.*]] : @closureCapture @guaranteed
447+
// CHECK-NEXT: project_box
448+
// CHECK-NEXT: // function_ref
449+
// CHECK-NEXT: function_ref
450+
// CHECK-NEXT: apply
451+
// CHECK-NEXT: begin_access
452+
// CHECK-NEXT: destroy_addr
453+
// CHECK-NEXT: store
454+
// CHECK-NEXT: end_access
455+
// CHECK-NEXT: tuple ()
456+
// CHECK-NEXT: return
457+
// CHECK-NEXT: } // end sil function 'closure_assignable_but_not_consumable_treated_like_inout'
458+
sil [ossa] @closure_assignable_but_not_consumable_treated_like_inout : $@convention(thin) (@guaranteed { var NonTrivialStruct }) -> () {
459+
bb0(%0 : @closureCapture @guaranteed ${ var NonTrivialStruct }):
460+
%4 = project_box %0 : ${ var NonTrivialStruct }, 0
461+
%9 = function_ref @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
462+
%10 = apply %9() : $@convention(thin) () -> @owned NonTrivialStruct
463+
%11 = begin_access [modify] [dynamic] %4 : $*NonTrivialStruct
464+
%12 = mark_must_check [assignable_but_not_consumable] %11 : $*NonTrivialStruct
465+
store %10 to [assign] %12 : $*NonTrivialStruct
466+
end_access %11 : $*NonTrivialStruct
467+
%24 = tuple ()
468+
return %24 : $()
469+
}

0 commit comments

Comments
 (0)