Skip to content

Commit bdea606

Browse files
committed
Mem2Reg: Fix lifetime completion for enum case values.
Enum types may have incomplete lifetimes in address form for trivial case values. When promoted to value form, they will end up with incomplete ossa lifetimes. Because we know that the incomplete enum values are trivial enum cases we complete their lifetimes with `end_lifetime` instead of `destroy_value`. This is especially important for Embedded Swift where we are not allowed to insert destroys which were not there before. Fixes a compiler crash in Embedded Swift caused by de-virtualizing such an inserted destroy and ending up with a non-specialized generic function. rdar://158807801
1 parent 81de885 commit bdea606

File tree

5 files changed

+132
-23
lines changed

5 files changed

+132
-23
lines changed

include/swift/SIL/OSSACompleteLifetime.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,22 @@ class OSSACompleteLifetime {
6262
/// TODO: Remove this option.
6363
bool ForceLivenessVerification;
6464

65+
/// If true, lifetimes are completed with `end_lifetime` instead of `destroy_value`.
66+
/// This can be used e.g. for enum values of non-trivial enum types, which are
67+
/// known to be a trivial case.
68+
bool nonDestroyingEnd;
69+
6570
public:
6671
OSSACompleteLifetime(
6772
SILFunction *function, const DominanceInfo *domInfo,
6873
DeadEndBlocks &deadEndBlocks,
6974
HandleTrivialVariable_t handleTrivialVariable = IgnoreTrivialVariable,
70-
bool forceLivenessVerification = false)
75+
bool forceLivenessVerification = false,
76+
bool nonDestroyingEnd = false)
7177
: domInfo(domInfo), deadEndBlocks(deadEndBlocks),
7278
completedValues(function), handleTrivialVariable(handleTrivialVariable),
73-
ForceLivenessVerification(forceLivenessVerification) {}
79+
ForceLivenessVerification(forceLivenessVerification),
80+
nonDestroyingEnd(nonDestroyingEnd) {}
7481

7582
/// The kind of boundary at which to complete the lifetime.
7683
///

lib/SIL/Utils/OSSACompleteLifetime.cpp

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ llvm::cl::opt<bool> VerifyLifetimeCompletion("verify-lifetime-completion",
7070

7171
static SILInstruction *endOSSALifetime(SILValue value,
7272
OSSACompleteLifetime::LifetimeEnd end,
73+
bool nonDestroyingEnd,
7374
SILBuilder &builder,
7475
DeadEndBlocks &deb) {
7576
auto loc =
@@ -82,7 +83,11 @@ static SILInstruction *endOSSALifetime(SILValue value,
8283
if (value->getType().is<SILBoxType>()) {
8384
return builder.createDeallocBox(loc, value, isDeadEnd);
8485
}
85-
return builder.createDestroyValue(loc, value, DontPoisonRefs, isDeadEnd);
86+
if (nonDestroyingEnd && !isDeadEnd) {
87+
return builder.createEndLifetime(loc, value);
88+
} else {
89+
return builder.createDestroyValue(loc, value, DontPoisonRefs, isDeadEnd);
90+
}
8691
}
8792
if (auto scopedAddress = ScopedAddressValue(value)) {
8893
return scopedAddress.createScopeEnd(builder.getInsertionPoint(), loc);
@@ -95,6 +100,7 @@ static SILInstruction *endOSSALifetime(SILValue value,
95100

96101
static bool endLifetimeAtLivenessBoundary(SILValue value,
97102
const SSAPrunedLiveness &liveness,
103+
bool nonDestroyingEnd,
98104
DeadEndBlocks &deb) {
99105
PrunedLivenessBoundary boundary;
100106
liveness.computeBoundary(boundary);
@@ -105,17 +111,17 @@ static bool endLifetimeAtLivenessBoundary(SILValue value,
105111
PrunedLiveness::LifetimeEndingUse) {
106112
changed = true;
107113
SILBuilderWithScope::insertAfter(
108-
lastUser, [value, &deb](SILBuilder &builder) {
114+
lastUser, [value, &deb, nonDestroyingEnd](SILBuilder &builder) {
109115
endOSSALifetime(value, OSSACompleteLifetime::LifetimeEnd::Boundary,
110-
builder, deb);
116+
nonDestroyingEnd, builder, deb);
111117
});
112118
}
113119
}
114120
for (SILBasicBlock *edge : boundary.boundaryEdges) {
115121
changed = true;
116122
SILBuilderWithScope builder(edge->begin());
117-
endOSSALifetime(value, OSSACompleteLifetime::LifetimeEnd::Boundary, builder,
118-
deb);
123+
endOSSALifetime(value, OSSACompleteLifetime::LifetimeEnd::Boundary,\
124+
nonDestroyingEnd, builder, deb);
119125
}
120126
for (SILNode *deadDef : boundary.deadDefs) {
121127
SILInstruction *next = nullptr;
@@ -126,8 +132,8 @@ static bool endLifetimeAtLivenessBoundary(SILValue value,
126132
}
127133
changed = true;
128134
SILBuilderWithScope builder(next);
129-
endOSSALifetime(value, OSSACompleteLifetime::LifetimeEnd::Boundary, builder,
130-
deb);
135+
endOSSALifetime(value, OSSACompleteLifetime::LifetimeEnd::Boundary,
136+
nonDestroyingEnd, builder, deb);
131137
}
132138
return changed;
133139
}
@@ -465,12 +471,13 @@ void OSSACompleteLifetime::visitAvailabilityBoundary(
465471

466472
static bool endLifetimeAtAvailabilityBoundary(SILValue value,
467473
const SSAPrunedLiveness &liveness,
474+
bool nonDestroyingEnd,
468475
DeadEndBlocks &deb) {
469476
bool changed = false;
470477
OSSACompleteLifetime::visitAvailabilityBoundary(
471478
value, liveness, [&](auto *unreachable, auto end) {
472479
SILBuilderWithScope builder(unreachable);
473-
endOSSALifetime(value, end, builder, deb);
480+
endOSSALifetime(value, end, nonDestroyingEnd, builder, deb);
474481
changed = true;
475482
});
476483
return changed;
@@ -479,15 +486,16 @@ static bool endLifetimeAtAvailabilityBoundary(SILValue value,
479486
static bool endLifetimeAtBoundary(SILValue value,
480487
SSAPrunedLiveness const &liveness,
481488
OSSACompleteLifetime::Boundary boundary,
489+
bool nonDestroyingEnd,
482490
DeadEndBlocks &deadEndBlocks) {
483491
bool changed = false;
484492
switch (boundary) {
485493
case OSSACompleteLifetime::Boundary::Liveness:
486-
changed |= endLifetimeAtLivenessBoundary(value, liveness, deadEndBlocks);
494+
changed |= endLifetimeAtLivenessBoundary(value, liveness, nonDestroyingEnd, deadEndBlocks);
487495
break;
488496
case OSSACompleteLifetime::Boundary::Availability:
489497
changed |=
490-
endLifetimeAtAvailabilityBoundary(value, liveness, deadEndBlocks);
498+
endLifetimeAtAvailabilityBoundary(value, liveness, nonDestroyingEnd, deadEndBlocks);
491499
break;
492500
}
493501
return changed;
@@ -548,7 +556,7 @@ bool OSSACompleteLifetime::analyzeAndUpdateLifetime(
548556
ASSERT(false && "caller must check for pointer escapes");
549557
}
550558
return endLifetimeAtBoundary(scopedAddress.value, liveness, boundary,
551-
deadEndBlocks);
559+
nonDestroyingEnd, deadEndBlocks);
552560
}
553561

554562
/// End the lifetime of \p value at unreachable instructions.
@@ -572,7 +580,7 @@ bool OSSACompleteLifetime::analyzeAndUpdateLifetime(SILValue value,
572580
LinearLiveness liveness(value);
573581
liveness.compute();
574582
return endLifetimeAtBoundary(value, liveness.getLiveness(), boundary,
575-
deadEndBlocks);
583+
nonDestroyingEnd, deadEndBlocks);
576584
}
577585
InteriorLiveness liveness(value);
578586
liveness.compute(domInfo, handleInnerScope);
@@ -586,7 +594,7 @@ bool OSSACompleteLifetime::analyzeAndUpdateLifetime(SILValue value,
586594
ASSERT(false && "caller must check for pointer escapes");
587595
}
588596
return endLifetimeAtBoundary(value, liveness.getLiveness(), boundary,
589-
deadEndBlocks);
597+
nonDestroyingEnd, deadEndBlocks);
590598
}
591599

592600
namespace swift::test {
@@ -598,16 +606,23 @@ namespace swift::test {
598606
static FunctionTest OSSACompleteLifetimeTest(
599607
"ossa_complete_lifetime", [](auto &function, auto &arguments, auto &test) {
600608
SILValue value = arguments.takeValue();
601-
OSSACompleteLifetime::Boundary kind =
602-
llvm::StringSwitch<OSSACompleteLifetime::Boundary>(
603-
arguments.takeString())
604-
.Case("liveness", OSSACompleteLifetime::Boundary::Liveness)
605-
.Case("availability",
606-
OSSACompleteLifetime::Boundary::Availability);
609+
OSSACompleteLifetime::Boundary kind = OSSACompleteLifetime::Boundary::Liveness;
610+
bool nonDestroyingEnd = false;
611+
StringRef kindArg = arguments.takeString();
612+
if (kindArg == "liveness_no_destroy") {
613+
nonDestroyingEnd = true;
614+
} else if (kindArg == "availability") {
615+
kind = OSSACompleteLifetime::Boundary::Availability;
616+
} else {
617+
assert(kindArg == "liveness");
618+
}
607619
auto *deb = test.getDeadEndBlocks();
608620
llvm::outs() << "OSSA lifetime completion on " << kind
609621
<< " boundary: " << value;
610-
OSSACompleteLifetime completion(&function, /*domInfo*/ nullptr, *deb);
622+
OSSACompleteLifetime completion(&function, /*domInfo*/ nullptr, *deb,
623+
OSSACompleteLifetime::IgnoreTrivialVariable,
624+
/*forceLivenessVerification=*/false,
625+
nonDestroyingEnd);
611626
completion.completeOSSALifetime(value, kind);
612627
function.print(llvm::outs());
613628
});

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1823,8 +1823,15 @@ void StackAllocationPromoter::run(BasicBlockSetVector &livePhiBlocks) {
18231823
deleter.forceDeleteWithUsers(asi);
18241824

18251825
// Now, complete lifetimes!
1826+
// We know that the incomplete values are trivial enum cases. Therefore we
1827+
// complete their lifetimes with `end_lifetime` instead of `destroy_value`.
1828+
// This is especially important for embedded swift where we are not allowed
1829+
// to insert destroys which were not there before.
18261830
OSSACompleteLifetime completion(function, domInfo,
1827-
*deadEndBlocksAnalysis->get(function));
1831+
*deadEndBlocksAnalysis->get(function),
1832+
OSSACompleteLifetime::IgnoreTrivialVariable,
1833+
/*forceLivenessVerification=*/false,
1834+
/*nonDestroyingEnd=*/true);
18281835

18291836
// We may have incomplete lifetimes for enum locations on trivial paths.
18301837
// After promoting them, complete lifetime here.

test/SILOptimizer/mem2reg_ossa_nontrivial.sil

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ enum KlassOptional {
5858
case none
5959
}
6060

61+
enum Result<S, F> {
62+
case success(S)
63+
case failure(F)
64+
}
65+
6166
sil [ossa] @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStruct
6267
sil [ossa] @get_nontrivialenum : $@convention(thin) () -> @owned NonTrivialEnum
6368
sil [ossa] @get_optionalnontrivialstruct : $@convention(thin) () -> @owned FakeOptional<NonTrivialStruct>
@@ -1549,3 +1554,40 @@ bb9:
15491554
dealloc_stack %5
15501555
br bb3
15511556
}
1557+
1558+
// CHECK-LABEL: sil [ossa] @test_end_lifetime_of_trivial_enum_case :
1559+
// CHECK-NOT: alloc_stack
1560+
// CHECK: bb2(%{{[0-9]+}} : $Int):
1561+
// CHECK: fix_lifetime
1562+
// CHECK: end_borrow
1563+
// CHECK: end_lifetime %1
1564+
// CHECK: bb3:
1565+
// CHECK-LABEL: } // end sil function 'test_end_lifetime_of_trivial_enum_case'
1566+
sil [ossa] @test_end_lifetime_of_trivial_enum_case : $@convention(thin) (@inout X, @owned Result<X, Int>) -> () {
1567+
bb0(%0 : $*X, %1 : @owned $Result<X, Int>):
1568+
%2 = alloc_stack $Result<X, Int>
1569+
store %1 to [init] %2
1570+
%3 = load_borrow %2
1571+
switch_enum %3, case #Result.success!enumelt: bb1, case #Result.failure!enumelt: bb2
1572+
1573+
bb1(%10 : @guaranteed $X):
1574+
end_borrow %3
1575+
%12 = load [take] %2
1576+
%13 = unchecked_enum_data %12 : $Result<X, Int>, #Result.success!enumelt
1577+
store %13 to [assign] %0
1578+
br bb3
1579+
1580+
bb2(%20 : $Int):
1581+
end_borrow %3
1582+
%22 = load_borrow %2
1583+
%23 = unchecked_enum_data %22, #Result.failure!enumelt
1584+
fix_lifetime %23
1585+
end_borrow %22
1586+
br bb3
1587+
1588+
bb3:
1589+
dealloc_stack %2
1590+
%r = tuple ()
1591+
return %r
1592+
}
1593+

test/SILOptimizer/ossa_lifetime_completion.sil

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ public enum FakeOptional<T> {
1717
case some(T)
1818
}
1919

20+
public enum E<T> {
21+
case a(T)
22+
case b(T)
23+
case c
24+
}
25+
2026
sil @swift_asyncLet_finish : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
2127
sil @swift_asyncLet_get : $@convention(thin) @async (Builtin.RawPointer, Builtin.RawPointer) -> ()
2228
sil @getC : $@convention(thin) () -> (@owned C)
@@ -123,6 +129,38 @@ bb3:
123129
return %r : $()
124130
}
125131

132+
// CHECK-LABEL: sil [ossa] @enumTest_no_destroy : $@convention(method) (@guaranteed E<C>) -> () {
133+
// CHECK: bb2(%{{[0-9]+}} : @guaranteed $C):
134+
// CHECK: destroy_value [dead_end]
135+
// CHECK-NEXT: unreachable
136+
// CHECK: bb3:
137+
// CHECK: end_lifetime
138+
// CHECK-NEXT: br bb4
139+
// CHECK-LABEL: } // end sil function 'enumTest_no_destroy'
140+
sil [ossa] @enumTest_no_destroy : $@convention(method) (@guaranteed E<C>) -> () {
141+
bb0(%0 : @guaranteed $E<C>):
142+
specify_test "ossa_complete_lifetime @instruction[0] liveness_no_destroy"
143+
%copy = copy_value %0 : $E<C>
144+
%borrow = begin_borrow %copy : $E<C>
145+
switch_enum %borrow : $E<C>, case #E.a!enumelt: bb1, case #E.b!enumelt: bb2, case #E.c!enumelt: bb3
146+
147+
bb1(%10 : @guaranteed $C):
148+
end_borrow %borrow : $E<C>
149+
destroy_value %copy : $E<C>
150+
br bb4
151+
152+
bb2(%15 : @guaranteed $C):
153+
unreachable
154+
155+
bb3:
156+
end_borrow %borrow : $E<C>
157+
br bb4
158+
159+
bb4:
160+
%r = tuple ()
161+
return %r : $()
162+
}
163+
126164
sil @use_guaranteed : $@convention(thin) (@guaranteed C) -> ()
127165

128166
sil [ossa] @argTest : $@convention(method) (@owned C) -> () {

0 commit comments

Comments
 (0)