Skip to content

Commit 4018910

Browse files
committed
[inlining] When devirting coroutines, place the end_borrow /after/ the end_apply, abort_apply.
Otherwise, we break ownership invariants. Looks like a thinko from when coroutines were added.
1 parent 8264009 commit 4018910

File tree

2 files changed

+87
-3
lines changed

2 files changed

+87
-3
lines changed

lib/SILOptimizer/Utils/Devirtualize.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,10 +571,11 @@ replaceBeginApplyInst(SILBuilder &builder, SILLocation loc,
571571
SILValue token = newBAI->getTokenResult();
572572

573573
// The token will only be used by end_apply and abort_apply. Use that to
574-
// insert the end_borrows we need.
574+
// insert the end_borrows we need /after/ those uses.
575575
for (auto *use : token->getUses()) {
576-
SILBuilderWithScope borrowBuilder(use->getUser(),
577-
builder.getBuilderContext());
576+
SILBuilderWithScope borrowBuilder(
577+
&*std::next(use->getUser()->getIterator()),
578+
builder.getBuilderContext());
578579
for (SILValue borrow : newArgBorrows) {
579580
borrowBuilder.createEndBorrow(loc, borrow);
580581
}

test/SILOptimizer/mandatory_inlining_ownership.sil

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ class C {
1010
init(i: Builtin.Int64)
1111
}
1212

13+
private class C2 {
14+
var i: Builtin.Int64 { get set }
15+
init(i: Builtin.Int64)
16+
}
17+
1318
class Klass {}
1419

1520
sil [transparent] [ossa] @calleeWithGuaranteed : $@convention(thin) (@guaranteed C) -> Builtin.Int64 {
@@ -501,3 +506,81 @@ bb3:
501506
return %19 : $()
502507
}
503508

509+
sil [ossa] [transparent] @devirt_callee : $@yield_once @convention(method) (@guaranteed C) -> @yields @inout Builtin.Int64 {
510+
bb0(%0 : @guaranteed $C):
511+
%1 = alloc_stack $Builtin.Int64
512+
%1a = integer_literal $Builtin.Int64, 0
513+
store %1a to [trivial] %1 : $*Builtin.Int64
514+
yield %1 : $*Builtin.Int64, resume bb1, unwind bb2
515+
516+
bb1:
517+
dealloc_stack %1 : $*Builtin.Int64
518+
%6 = tuple ()
519+
return %6 : $()
520+
521+
bb2:
522+
dealloc_stack %1 : $*Builtin.Int64
523+
unwind
524+
}
525+
526+
// Just make sure we actually inlined the begin_apply. We just want to make sure
527+
// we are not breaking ownership invariants by not properly borrowing %0.
528+
// CHECK-LABEL: sil [ossa] @begin_apply_devirt_caller : $@convention(method) (@owned C2) -> @error Error {
529+
// CHECK-NOT: begin_apply
530+
// CHECK: } // end sil function 'begin_apply_devirt_caller'
531+
sil [ossa] @begin_apply_devirt_caller : $@convention(method) (@owned C2) -> @error Error {
532+
bb0(%0 : @owned $C2):
533+
%1 = class_method %0 : $C2, #C2.i!modify.1 : (C2) -> () -> (), $@yield_once @convention(method) (@guaranteed C2) -> @yields @inout Builtin.Int64
534+
(%mem, %tok) = begin_apply %1(%0) : $@yield_once @convention(method) (@guaranteed C2) -> @yields @inout Builtin.Int64
535+
br bb1
536+
537+
bb1:
538+
end_apply %tok
539+
destroy_value %0 : $C2
540+
%9999 = tuple()
541+
return %9999 : $()
542+
}
543+
544+
// CHECK-LABEL: sil [ossa] @begin_apply_devirt_caller_2 : $@convention(method) (@owned C2) -> @error Error {
545+
// CHECK-NOT: begin_apply
546+
// CHECK: } // end sil function 'begin_apply_devirt_caller_2'
547+
sil [ossa] @begin_apply_devirt_caller_2 : $@convention(method) (@owned C2) -> @error Error {
548+
bb0(%0 : @owned $C2):
549+
%1 = class_method %0 : $C2, #C2.i!modify.1 : (C2) -> () -> (), $@yield_once @convention(method) (@guaranteed C2) -> @yields @inout Builtin.Int64
550+
(%mem, %tok) = begin_apply %1(%0) : $@yield_once @convention(method) (@guaranteed C2) -> @yields @inout Builtin.Int64
551+
br bb1
552+
553+
bb1:
554+
abort_apply %tok
555+
destroy_value %0 : $C2
556+
%9999 = tuple()
557+
return %9999 : $()
558+
}
559+
560+
// CHECK-LABEL: sil [ossa] @begin_apply_devirt_caller_3 : $@convention(method) (@owned C2) -> @error Error {
561+
// CHECK-NOT: begin_apply
562+
// CHECK: } // end sil function 'begin_apply_devirt_caller_3'
563+
sil [ossa] @begin_apply_devirt_caller_3 : $@convention(method) (@owned C2) -> @error Error {
564+
bb0(%0 : @owned $C2):
565+
%1 = class_method %0 : $C2, #C2.i!modify.1 : (C2) -> () -> (), $@yield_once @convention(method) (@guaranteed C2) -> @yields @inout Builtin.Int64
566+
(%mem, %tok) = begin_apply %1(%0) : $@yield_once @convention(method) (@guaranteed C2) -> @yields @inout Builtin.Int64
567+
cond_br undef, bb1, bb2
568+
569+
bb1:
570+
end_apply %tok
571+
br bb3
572+
573+
bb2:
574+
abort_apply %tok
575+
br bb3
576+
577+
bb3:
578+
destroy_value %0 : $C2
579+
%9999 = tuple()
580+
return %9999 : $()
581+
}
582+
583+
584+
sil_vtable C2 {
585+
#C2.i!modify.1: (C2) -> () -> () : @devirt_callee
586+
}

0 commit comments

Comments
 (0)