Skip to content

Commit 969d776

Browse files
gottesmmgottesmm
committed
[allocbox-to-stack] When we promote a heap -> stack of noncopyable type, move mark_must_check onto alloc_stack and always
ensure that we use consumable_to_assign. What this patch is does is add an extra phase after alloc_box runs where we look at uses of the alloc_stack and if we see any mark_must_check of any kind, we delete them and rewrite a single mark_must_check [consumable_and_assignable] on the alloc_stack and make all uses of the alloc_stack go through the mark_must_check. This has two effects: 1. In a subsequent PR when I add noncopyable semantics for escaping closures, this will cause allocbox to stack to convert such boxes from having escaping semantics to having non-escaping semantics. Escaping semantics means that we always reproject out from the box and use mark_must_check [assignable_but_not_consumable] (since we can't consume from the box, but can assign to it). In contrast, non-escaping semantics means that the box becomes an alloc_stack and we use the traditional var checker semantics. NOTE: We can do this for lets represented as addresses and vars since the typechecker will validate that the let is never actually written to even if at the SIL level we would allow that. 2. In cases where we are implementing simple mark_must_check [consumable_and_assignable] on one of the project_box and capture the box, we used to have a problem where the direct box uses would be on the alloc_stack and not go through the mark_must_check. Now, all uses after allocbox_to_stack occur go through the mark_must_check. This is why I was able to remove instances of the "compiler does not understand this pattern" errors... since the compiler with this change can now understand them.
1 parent 5bc3b99 commit 969d776

File tree

5 files changed

+171
-111
lines changed

5 files changed

+171
-111
lines changed

‎lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "swift/SIL/ApplySite.h"
1919
#include "swift/SIL/BasicBlockDatastructures.h"
2020
#include "swift/SIL/Dominance.h"
21+
#include "swift/SIL/MemAccessUtils.h"
2122
#include "swift/SIL/SILArgument.h"
2223
#include "swift/SIL/SILBuilder.h"
2324
#include "swift/SIL/SILCloner.h"
@@ -500,25 +501,70 @@ struct AllocBoxToStackState {
500501
};
501502
} // anonymous namespace
502503

503-
static void replaceProjectBoxUsers(SILValue HeapBox, SILValue StackBox) {
504-
SmallVector<Operand *, 8> Worklist(HeapBox->use_begin(), HeapBox->use_end());
505-
while (!Worklist.empty()) {
506-
auto *Op = Worklist.pop_back_val();
507-
if (auto *PBI = dyn_cast<ProjectBoxInst>(Op->getUser())) {
504+
static void replaceProjectBoxUsers(SILValue heapBox, SILValue stackBox) {
505+
StackList<Operand *> worklist(heapBox->getFunction());
506+
for (auto *use : heapBox->getUses())
507+
worklist.push_back(use);
508+
while (!worklist.empty()) {
509+
auto *nextUse = worklist.pop_back_val();
510+
if (auto *pbi = dyn_cast<ProjectBoxInst>(nextUse->getUser())) {
508511
// This may result in an alloc_stack being used by begin_access [dynamic].
509-
PBI->replaceAllUsesWith(StackBox);
512+
pbi->replaceAllUsesWith(stackBox);
513+
pbi->eraseFromParent();
510514
continue;
511515
}
512516

513-
auto *User = Op->getUser();
514-
if (isa<MarkUninitializedInst>(User) || isa<CopyValueInst>(User) ||
515-
isa<BeginBorrowInst>(User)) {
516-
llvm::copy(cast<SingleValueInstruction>(User)->getUses(),
517-
std::back_inserter(Worklist));
517+
auto *user = nextUse->getUser();
518+
if (isa<MarkUninitializedInst>(user) || isa<CopyValueInst>(user) ||
519+
isa<BeginBorrowInst>(user)) {
520+
for (auto *use : cast<SingleValueInstruction>(user)->getUses()) {
521+
worklist.push_back(use);
522+
}
518523
}
519524
}
520525
}
521526

527+
static void hoistMarkMustCheckInsts(SingleValueInstruction *stackBox) {
528+
StackList<Operand *> worklist(stackBox->getFunction());
529+
530+
for (auto *use : stackBox->getUses()) {
531+
worklist.push_back(use);
532+
}
533+
534+
bool foundTarget = false;
535+
while (!worklist.empty()) {
536+
auto *nextUse = worklist.pop_back_val();
537+
auto *nextUser = nextUse->getUser();
538+
539+
if (isa<BeginBorrowInst>(nextUser) || isa<BeginAccessInst>(nextUser) ||
540+
isa<CopyValueInst>(nextUser) || isa<MarkUninitializedInst>(nextUser)) {
541+
for (auto result : nextUser->getResults()) {
542+
for (auto *use : result->getUses())
543+
worklist.push_back(use);
544+
}
545+
}
546+
547+
if (auto *mmci = dyn_cast<MarkMustCheckInst>(nextUser)) {
548+
foundTarget = true;
549+
mmci->replaceAllUsesWith(mmci->getOperand());
550+
mmci->eraseFromParent();
551+
}
552+
}
553+
554+
if (!foundTarget)
555+
return;
556+
557+
SILBuilderWithScope builder(stackBox);
558+
builder.insertAfter(stackBox, [&](SILBuilder &b) {
559+
auto *undef = SILUndef::get(stackBox->getType(), stackBox->getModule());
560+
auto *mmci = b.createMarkMustCheckInst(
561+
stackBox->getLoc(), undef,
562+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
563+
stackBox->replaceAllUsesWith(mmci);
564+
mmci->setOperand(stackBox);
565+
});
566+
}
567+
522568
/// rewriteAllocBoxAsAllocStack - Replace uses of the alloc_box with a
523569
/// new alloc_stack, but do not delete the alloc_box yet.
524570
static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
@@ -574,7 +620,7 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
574620
ABI->hasDynamicLifetime(), isLexical());
575621

576622
// Transfer a mark_uninitialized if we have one.
577-
SILValue StackBox = ASI;
623+
SingleValueInstruction *StackBox = ASI;
578624
if (Kind) {
579625
StackBox =
580626
Builder.createMarkUninitialized(ASI->getLoc(), ASI, Kind.value());
@@ -584,6 +630,12 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
584630
// the address of the stack location.
585631
replaceProjectBoxUsers(HeapBox, StackBox);
586632

633+
// Then hoist any mark_must_check [assignable_but_not_consumable] to the
634+
// alloc_stack and convert them to [consumable_but_not_assignable]. This is
635+
// because we are semantically converting from escaping semantics to
636+
// non-escaping semantics.
637+
hoistMarkMustCheckInsts(StackBox);
638+
587639
assert(ABI->getBoxType()->getLayout()->getFields().size() == 1
588640
&& "promoting multi-field box not implemented");
589641
auto &Lowering = ABI->getFunction()->getTypeLowering(

‎test/SILOptimizer/allocbox_to_stack_noncopyable.sil

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// RUN: %target-sil-opt -enable-experimental-move-only -enable-sil-verify-all %s -allocbox-to-stack -enable-copy-propagation=requested-passes-only -enable-lexical-borrow-scopes=false | %FileCheck %s
2+
3+
sil_stage raw
4+
5+
import Builtin
6+
7+
@_moveOnly
8+
struct NonTrivialStruct {
9+
var i: Builtin.Int32
10+
}
11+
12+
sil @use_nontrivialstruct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
13+
14+
sil [ossa] @host_markmustcheck_closure : $@convention(thin) (@guaranteed { var NonTrivialStruct }) -> () {
15+
bb0(%0 : @closureCapture @guaranteed ${ var NonTrivialStruct }):
16+
%9999 = tuple()
17+
return %9999 : $()
18+
}
19+
20+
// CHECK-LABEL: sil [ossa] @hoist_markmustcheck : $@convention(thin) (@owned NonTrivialStruct) -> () {
21+
// CHECK: bb0([[ARG:%.*]] : @owned
22+
// CHECK-NEXT: [[STACK1:%.*]] = alloc_stack $NonTrivialStruct
23+
// CHECK-NEXT: [[MARKED1:%.*]] = mark_must_check [consumable_and_assignable] [[STACK1]]
24+
// CHECK-NEXT: store [[ARG]] to [init] [[MARKED1]]
25+
// CHECK-NEXT: [[LOADED_VALUE:%.*]] = load [take] [[MARKED1]]
26+
// CHECK-NEXT: [[STACK2:%.*]] = alloc_stack $NonTrivialStruct
27+
// CHECK-NEXT: [[MARKED2:%.*]] = mark_must_check [consumable_and_assignable] [[STACK2]]
28+
// CHECK-NEXT: store [[LOADED_VALUE]] to [init] [[MARKED2]]
29+
// CHECK-NEXT: // function_ref
30+
// CHECK-NEXT: [[FUNC:%.*]] = function_ref @$s26host_markmustcheck_closureTf0s_n : $@convention(thin) (@inout_aliasable NonTrivialStruct) -> ()
31+
// CHECK-NEXT: [[PA:%.*]] = partial_apply [callee_guaranteed] [[FUNC]]([[MARKED2]]) : $@convention(thin) (@inout_aliasable NonTrivialStruct) -> ()
32+
// CHECK-NEXT: apply [[PA]]()
33+
// CHECK-NEXT: destroy_value [[PA]]
34+
// CHECK-NEXT: destroy_addr [[MARKED2]]
35+
// CHECK-NEXT: dealloc_stack [[STACK2]]
36+
// CHECK-NEXT: dealloc_stack [[STACK1]]
37+
// CHECK-NEXT: tuple ()
38+
// CHECK-NEXT: return
39+
// CHECK: } // end sil function 'hoist_markmustcheck'
40+
sil [ossa] @hoist_markmustcheck : $@convention(thin) (@owned NonTrivialStruct) -> () {
41+
bb0(%0 : @owned $NonTrivialStruct):
42+
%1 = alloc_box ${ var NonTrivialStruct }
43+
%2 = project_box %1 : ${ var NonTrivialStruct }, 0
44+
store %0 to [init] %2 : $*NonTrivialStruct
45+
46+
%3 = project_box %1 : ${ var NonTrivialStruct }, 0
47+
%3a = mark_must_check [assignable_but_not_consumable] %3 : $*NonTrivialStruct
48+
%3b = load [take] %3a : $*NonTrivialStruct
49+
50+
%4 = alloc_box ${ var NonTrivialStruct }
51+
%4a = project_box %4 : ${ var NonTrivialStruct }, 0
52+
%4b = mark_must_check [assignable_but_not_consumable] %4a : $*NonTrivialStruct
53+
store %3b to [init] %4b : $*NonTrivialStruct
54+
55+
%f = function_ref @host_markmustcheck_closure : $@convention(thin) (@guaranteed { var NonTrivialStruct }) -> ()
56+
%5c = copy_value %4 : ${ var NonTrivialStruct }
57+
%g = partial_apply [callee_guaranteed] %f(%5c) : $@convention(thin) (@guaranteed { var NonTrivialStruct }) -> ()
58+
apply %g() : $@callee_guaranteed () -> ()
59+
60+
destroy_value %g : $@callee_guaranteed () -> ()
61+
destroy_value %4 : ${ var NonTrivialStruct }
62+
dealloc_box %1 : ${ var NonTrivialStruct }
63+
%9999 = tuple()
64+
return %9999 : $()
65+
}

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.sil

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ bb0(%arg : @owned $AggStruct):
105105
// expected-error @-1 {{'x2' consumed more than once}}
106106
// expected-error @-2 {{'x2' consumed by a use in a loop}}
107107
%9 = begin_access [modify] [static] %1 : $*AggStruct
108-
store %arg to [assign] %9 : $*AggStruct
108+
store %arg to [init] %9 : $*AggStruct
109109
end_access %9 : $*AggStruct
110110
%12 = begin_access [read] [static] %1 : $*AggStruct
111111
%13 = struct_element_addr %12 : $*AggStruct, #AggStruct.pair
@@ -204,54 +204,6 @@ bb0(%arg : @owned $NonTrivialStruct, %arg1 : @owned $NonTrivialStruct):
204204
return %33 : $()
205205
}
206206

207-
// In this case, we already emitted an object error earlier so all of the
208-
// copy_value are explicit_copy_value. We shouldn't crash when processing
209-
// this. We used to due to borrow to destructure trying to delete
210-
// %14->getOperand(Src)->getDefiningInstruction() and not handling the case
211-
// where %10 was an argument.
212-
sil [ossa] @enumPatternMatchSwitch : $@convention(thin) (@guaranteed EnumTy) -> () {
213-
bb0(%0 : @guaranteed $EnumTy):
214-
%1 = explicit_copy_value %0 : $EnumTy
215-
debug_value %1 : $EnumTy, let, name "x", argno 1
216-
%3 = alloc_box ${ var EnumTy }, let, name "x2"
217-
%4 = project_box %3 : ${ var EnumTy }, 0
218-
%5 = mark_must_check [consumable_and_assignable] %4 : $*EnumTy // expected-error {{'x2' used after consume}}
219-
store %1 to [init] %5 : $*EnumTy
220-
%7 = load [copy] %5 : $*EnumTy // expected-note {{consuming use here}}
221-
%8 = begin_borrow %7 : $EnumTy
222-
switch_enum %8 : $EnumTy, case #EnumTy.klass!enumelt: bb1, case #EnumTy.int!enumelt: bb2
223-
224-
bb1(%10 : @guaranteed $NonTrivialStruct):
225-
%11 = alloc_box ${ var NonTrivialStruct }, let, name "k"
226-
%12 = project_box %11 : ${ var NonTrivialStruct }, 0
227-
%13 = mark_must_check [consumable_and_assignable] %12 : $*NonTrivialStruct
228-
%14 = explicit_copy_value %10 : $NonTrivialStruct
229-
store %14 to [init] %13 : $*NonTrivialStruct
230-
debug_value %13 : $*NonTrivialStruct, let, name "k", expr op_deref
231-
%17 = load_borrow %13 : $*NonTrivialStruct
232-
%18 = function_ref @nontrivialstruct_use : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
233-
%19 = apply %18(%17) : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
234-
end_borrow %17 : $NonTrivialStruct
235-
%21 = load_borrow %5 : $*EnumTy // expected-note {{non-consuming use here}}
236-
%22 = function_ref @enumty_use : $@convention(thin) (@guaranteed EnumTy) -> ()
237-
%23 = apply %22(%21) : $@convention(thin) (@guaranteed EnumTy) -> ()
238-
end_borrow %21 : $EnumTy
239-
destroy_value %11 : ${ var NonTrivialStruct }
240-
end_borrow %8 : $EnumTy
241-
destroy_value %7 : $EnumTy
242-
br bb3
243-
244-
bb2(%28 : $Builtin.Int32):
245-
end_borrow %8 : $EnumTy
246-
destroy_value %7 : $EnumTy
247-
br bb3
248-
249-
bb3:
250-
destroy_value %3 : ${ var EnumTy }
251-
%33 = tuple ()
252-
return %33 : $()
253-
}
254-
255207
///////////////////////////////////////////////////////////////////////
256208
// MARK: Tests that make sure we emit a nice error for missed copies //
257209
///////////////////////////////////////////////////////////////////////

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,9 +1896,8 @@ public func closureCaptureClassUseAfterConsume() {
18961896
var x2 = Klass()
18971897
// expected-error @-1 {{'x2' consumed more than once}}
18981898
// expected-error @-2 {{'x2' consumed more than once}}
1899-
// expected-error @-3 {{Usage of a move only type that the move checker does not know how to check!}}
1899+
// expected-error @-3 {{'x2' consumed in closure but not reinitialized before end of closure}}
19001900
// expected-error @-4 {{'x2' consumed in closure but not reinitialized before end of closure}}
1901-
// expected-error @-5 {{'x2' consumed in closure but not reinitialized before end of closure}}
19021901
x2 = Klass()
19031902
let f = {
19041903
borrowVal(x2)
@@ -1916,9 +1915,8 @@ public func closureCaptureClassUseAfterConsume() {
19161915

19171916
public func closureCaptureClassUseAfterConsume2() {
19181917
var x2 = Klass()
1919-
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
1918+
// expected-error @-1 {{'x2' consumed in closure but not reinitialized before end of closure}}
19201919
// expected-error @-2 {{'x2' consumed in closure but not reinitialized before end of closure}}
1921-
// expected-error @-3 {{'x2' consumed in closure but not reinitialized before end of closure}}
19221920
x2 = Klass()
19231921
let f = {
19241922
borrowVal(x2)
@@ -1931,13 +1929,13 @@ public func closureCaptureClassUseAfterConsume2() {
19311929

19321930
public func closureCaptureClassUseAfterConsumeError() {
19331931
var x2 = Klass()
1934-
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
1932+
// expected-error @-1 {{'x2' consumed more than once}}
19351933
// expected-error @-2 {{'x2' consumed more than once}}
19361934
// expected-error @-3 {{'x2' consumed more than once}}
19371935
// expected-error @-4 {{'x2' consumed in closure but not reinitialized before end of closure}}
19381936
// expected-error @-5 {{'x2' consumed in closure but not reinitialized before end of closure}}
19391937
x2 = Klass()
1940-
let f = {
1938+
let f = { // expected-note {{consuming use here}}
19411939
borrowVal(x2)
19421940
consumeVal(x2)
19431941
// expected-note @-1 {{consuming use here}}
@@ -1949,7 +1947,7 @@ public func closureCaptureClassUseAfterConsumeError() {
19491947
// expected-note @-4 {{consuming use here}}
19501948
}
19511949
f()
1952-
let x3 = x2
1950+
let x3 = x2 // expected-note {{consuming use here}}
19531951
let _ = x3
19541952
}
19551953

@@ -2017,9 +2015,8 @@ public func deferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
20172015

20182016
public func closureAndDeferCaptureClassUseAfterConsume() {
20192017
var x2 = Klass()
2020-
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
2021-
// expected-error @-2 {{'x2' consumed in closure but not reinitialized before end of closure}}
2022-
// expected-error @-3 {{'x2' consumed more than once}}
2018+
// expected-error @-1 {{'x2' consumed in closure but not reinitialized before end of closure}}
2019+
// expected-error @-2 {{'x2' consumed more than once}}
20232020
x2 = Klass()
20242021
let f = {
20252022
defer {
@@ -2037,10 +2034,9 @@ public func closureAndDeferCaptureClassUseAfterConsume() {
20372034
public func closureAndDeferCaptureClassUseAfterConsume2() {
20382035
var x2 = Klass()
20392036
// expected-error @-1 {{'x2' consumed in closure but not reinitialized before end of closure}}
2040-
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
2041-
// expected-error @-3 {{'x2' consumed more than once}}
2037+
// expected-error @-2 {{'x2' consumed more than once}}
2038+
// expected-error @-3 {{'x2' used after consume}}
20422039
// expected-error @-4 {{'x2' used after consume}}
2043-
// expected-error @-5 {{'x2' used after consume}}
20442040
x2 = Klass()
20452041
let f = {
20462042
consumeVal(x2)
@@ -2063,12 +2059,12 @@ public func closureAndDeferCaptureClassUseAfterConsume2() {
20632059
public func closureAndDeferCaptureClassUseAfterConsume3() {
20642060
var x2 = Klass()
20652061
// expected-error @-1 {{'x2' consumed in closure but not reinitialized before end of closure}}
2066-
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
2062+
// expected-error @-2 {{'x2' consumed more than once}}
20672063
// expected-error @-3 {{'x2' consumed more than once}}
20682064
// expected-error @-4 {{'x2' used after consume}}
20692065
// expected-error @-5 {{'x2' used after consume}}
20702066
x2 = Klass()
2071-
let f = {
2067+
let f = { // expected-note {{consuming use here}}
20722068
consumeVal(x2)
20732069
// expected-note @-1 {{consuming use here}}
20742070
// expected-note @-2 {{consuming use here}}
@@ -2084,7 +2080,7 @@ public func closureAndDeferCaptureClassUseAfterConsume3() {
20842080
print("foo")
20852081
}
20862082
f()
2087-
consumeVal(x2)
2083+
consumeVal(x2) // expected-note {{consuming use here}}
20882084
}
20892085

20902086
public func closureAndDeferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
@@ -2110,11 +2106,10 @@ public func closureAndDeferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
21102106
public func closureAndClosureCaptureClassUseAfterConsume() {
21112107
var x2 = Klass()
21122108
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
2113-
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
2114-
// expected-error @-3 {{'x2' consumed more than once}}
2115-
// expected-error @-4 {{'x2' consumed in closure but not reinitialized before end of closure}}
2116-
// expected-error @-5 {{'x2' consumed more than once}}
2117-
// expected-error @-6 {{'x2' consumed in closure but not reinitialized before end of closure}}
2109+
// expected-error @-2 {{'x2' consumed more than once}}
2110+
// expected-error @-3 {{'x2' consumed in closure but not reinitialized before end of closure}}
2111+
// expected-error @-4 {{'x2' consumed more than once}}
2112+
// expected-error @-5 {{'x2' consumed in closure but not reinitialized before end of closure}}
21182113
x2 = Klass()
21192114
let f = {
21202115
let g = {
@@ -2136,13 +2131,13 @@ public func closureAndClosureCaptureClassUseAfterConsume() {
21362131
public func closureAndClosureCaptureClassUseAfterConsume2() {
21372132
var x2 = Klass()
21382133
// expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
2139-
// expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
2134+
// expected-error @-2 {{'x2' consumed more than once}}
21402135
// expected-error @-3 {{'x2' consumed in closure but not reinitialized before end of closure}}
21412136
// expected-error @-4 {{'x2' consumed more than once}}
21422137
// expected-error @-5 {{'x2' consumed more than once}}
21432138
// expected-error @-6 {{'x2' consumed in closure but not reinitialized before end of closure}}
21442139
x2 = Klass()
2145-
let f = {
2140+
let f = { // expected-note {{consuming use here}}
21462141
let g = {
21472142
borrowVal(x2)
21482143
consumeVal(x2)
@@ -2157,7 +2152,7 @@ public func closureAndClosureCaptureClassUseAfterConsume2() {
21572152
g()
21582153
}
21592154
f()
2160-
consumeVal(x2)
2155+
consumeVal(x2) // expected-note {{consuming use here}}
21612156
}
21622157

21632158

0 commit comments

Comments
 (0)