Skip to content

Commit 762cdc4

Browse files
atrickatrick
committed
Fix PredictableDeadAllocationElimination ownership for empty structs
Preserve ownership for empty non-trivial structs. This currently applies to ~Escapable structs. People often use empty structs to investigate language behavior. They should behave just like a struct that wraps a pointer. Previously, this would crash later during OSSA lifetime completion: Assertion failed: (isa<UnreachableInst>(block->getTerminator())), function computeRegion, file OSSALifetimeCompletion.cpp.
1 parent 37a9e54 commit 762cdc4

File tree

2 files changed

+61
-26
lines changed

2 files changed

+61
-26
lines changed

‎lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -91,23 +91,28 @@ getFullyReferenceableStruct(SILType Ty) {
9191
return SD;
9292
}
9393

94-
static unsigned getNumSubElements(SILType T, SILModule &M,
94+
static unsigned getNumSubElements(SILType T, SILFunction &F,
9595
TypeExpansionContext context) {
9696

9797
if (auto TT = T.getAs<TupleType>()) {
9898
unsigned NumElements = 0;
9999
for (auto index : indices(TT.getElementTypes()))
100100
NumElements +=
101-
getNumSubElements(T.getTupleElementType(index), M, context);
101+
getNumSubElements(T.getTupleElementType(index), F, context);
102102
return NumElements;
103103
}
104104

105105
if (auto *SD = getFullyReferenceableStruct(T)) {
106106
unsigned NumElements = 0;
107107
for (auto *D : SD->getStoredProperties())
108108
NumElements +=
109-
getNumSubElements(T.getFieldType(D, M, context), M, context);
110-
return NumElements;
109+
getNumSubElements(T.getFieldType(D, F.getModule(), context), F,
110+
context);
111+
112+
// Returning NumElements == 0 implies that "empty" values can be
113+
// materialized without ownership. This is only valid for trivial types.
114+
if (NumElements > 0 || T.isTrivial(F))
115+
return NumElements;
111116
}
112117

113118
// If this isn't a tuple or struct, it is a single element.
@@ -152,7 +157,7 @@ static SILValue getAccessPathRoot(SILValue pointer) {
152157
static unsigned computeSubelement(SILValue Pointer,
153158
SingleValueInstruction *RootInst) {
154159
unsigned SubElementNumber = 0;
155-
SILModule &M = RootInst->getModule();
160+
auto &F = *RootInst->getFunction();
156161

157162
while (1) {
158163
// If we got to the root, we're done.
@@ -175,7 +180,7 @@ static unsigned computeSubelement(SILValue Pointer,
175180
// Keep track of what subelement is being referenced.
176181
for (unsigned i = 0, e = TEAI->getFieldIndex(); i != e; ++i) {
177182
SubElementNumber +=
178-
getNumSubElements(TT.getTupleElementType(i), M,
183+
getNumSubElements(TT.getTupleElementType(i), F,
179184
TypeExpansionContext(*RootInst->getFunction()));
180185
}
181186
Pointer = TEAI->getOperand();
@@ -191,7 +196,8 @@ static unsigned computeSubelement(SILValue Pointer,
191196
if (D == SEAI->getField()) break;
192197
auto context = TypeExpansionContext(*RootInst->getFunction());
193198
SubElementNumber +=
194-
getNumSubElements(ST.getFieldType(D, M, context), M, context);
199+
getNumSubElements(ST.getFieldType(D, F.getModule(), context), F,
200+
context);
195201
}
196202

197203
Pointer = SEAI->getOperand();
@@ -370,10 +376,9 @@ static bool isFullyAvailable(SILType loadTy, unsigned firstElt,
370376
if (!firstVal || firstVal.getType() != loadTy)
371377
return false;
372378

373-
auto *function = firstVal.getValue()->getFunction();
379+
auto &function = *firstVal.getValue()->getFunction();
374380
return llvm::all_of(
375-
range(getNumSubElements(loadTy, function->getModule(),
376-
TypeExpansionContext(*function))),
381+
range(getNumSubElements(loadTy, function, TypeExpansionContext(function))),
377382
[&](unsigned index) -> bool {
378383
auto &val = AvailableValues[firstElt + index];
379384
return val.getValue() == firstVal.getValue() &&
@@ -395,7 +400,7 @@ static SILValue nonDestructivelyExtractSubElement(const AvailableValue &Val,
395400
// Keep track of what subelement is being referenced.
396401
SILType EltTy = ValTy.getTupleElementType(EltNo);
397402
unsigned NumSubElt = getNumSubElements(
398-
EltTy, B.getModule(), TypeExpansionContext(B.getFunction()));
403+
EltTy, B.getFunction(), TypeExpansionContext(B.getFunction()));
399404
if (SubElementNumber < NumSubElt) {
400405
auto BorrowedVal = Val.emitBeginBorrow(B, Loc);
401406
auto NewVal =
@@ -420,7 +425,7 @@ static SILValue nonDestructivelyExtractSubElement(const AvailableValue &Val,
420425
auto fieldType = ValTy.getFieldType(
421426
D, B.getModule(), TypeExpansionContext(B.getFunction()));
422427
unsigned NumSubElt = getNumSubElements(
423-
fieldType, B.getModule(), TypeExpansionContext(B.getFunction()));
428+
fieldType, B.getFunction(), TypeExpansionContext(B.getFunction()));
424429

425430
if (SubElementNumber < NumSubElt) {
426431
auto BorrowedVal = Val.emitBeginBorrow(B, Loc);
@@ -1227,7 +1232,8 @@ bool AvailableValueAggregator::canTake(SILType loadTy,
12271232
return llvm::all_of(indices(tt->getElements()), [&](unsigned eltNo) {
12281233
SILType eltTy = loadTy.getTupleElementType(eltNo);
12291234
unsigned numSubElt =
1230-
getNumSubElements(eltTy, M, TypeExpansionContext(B.getFunction()));
1235+
getNumSubElements(eltTy, B.getFunction(),
1236+
TypeExpansionContext(B.getFunction()));
12311237
bool success = canTake(eltTy, firstElt);
12321238
firstElt += numSubElt;
12331239
return success;
@@ -1238,7 +1244,7 @@ bool AvailableValueAggregator::canTake(SILType loadTy,
12381244
return llvm::all_of(sd->getStoredProperties(), [&](VarDecl *decl) -> bool {
12391245
auto context = TypeExpansionContext(B.getFunction());
12401246
SILType eltTy = loadTy.getFieldType(decl, M, context);
1241-
unsigned numSubElt = getNumSubElements(eltTy, M, context);
1247+
unsigned numSubElt = getNumSubElements(eltTy, B.getFunction(), context);
12421248
bool success = canTake(eltTy, firstElt);
12431249
firstElt += numSubElt;
12441250
return success;
@@ -1369,7 +1375,8 @@ SILValue AvailableValueAggregator::aggregateTupleSubElts(TupleType *TT,
13691375
for (unsigned EltNo : indices(TT->getElements())) {
13701376
SILType EltTy = LoadTy.getTupleElementType(EltNo);
13711377
unsigned NumSubElt =
1372-
getNumSubElements(EltTy, M, TypeExpansionContext(B.getFunction()));
1378+
getNumSubElements(EltTy, B.getFunction(),
1379+
TypeExpansionContext(B.getFunction()));
13731380

13741381
// If we are missing any of the available values in this struct element,
13751382
// compute an address to load from.
@@ -1406,7 +1413,7 @@ SILValue AvailableValueAggregator::aggregateStructSubElts(StructDecl *sd,
14061413
for (auto *decl : sd->getStoredProperties()) {
14071414
auto context = TypeExpansionContext(B.getFunction());
14081415
SILType eltTy = loadTy.getFieldType(decl, M, context);
1409-
unsigned numSubElt = getNumSubElements(eltTy, M, context);
1416+
unsigned numSubElt = getNumSubElements(eltTy, B.getFunction(), context);
14101417

14111418
// If we are missing any of the available values in this struct element,
14121419
// compute an address to load from.
@@ -1540,6 +1547,8 @@ class AvailableValueDataflowContext {
15401547
private:
15411548
SILModule &getModule() const { return TheMemory->getModule(); }
15421549

1550+
SILFunction &getFunction() const { return *TheMemory->getFunction(); }
1551+
15431552
void updateAvailableValues(
15441553
SILInstruction *Inst,
15451554
SmallBitVector &RequiredElts,
@@ -1673,7 +1682,7 @@ AvailableValueDataflowContext::computeAvailableValues(
16731682
return std::nullopt;
16741683

16751684
unsigned NumLoadSubElements = getNumSubElements(
1676-
LoadTy, getModule(), TypeExpansionContext(*TheMemory->getFunction()));
1685+
LoadTy, getFunction(), TypeExpansionContext(getFunction()));
16771686

16781687
LoadInfo loadInfo = {LoadTy, FirstElt, NumLoadSubElements};
16791688

@@ -1712,14 +1721,14 @@ static inline void updateAvailableValuesHelper(
17121721
SmallBitVector &conflictingValues,
17131722
function_ref<std::optional<AvailableValue>(unsigned)> defaultFunc,
17141723
function_ref<bool(AvailableValue &, unsigned)> isSafeFunc) {
1715-
auto &mod = theMemory->getModule();
17161724
unsigned startSubElt = computeSubelement(address, theMemory);
17171725

17181726
// TODO: Is this needed now?
17191727
assert(startSubElt != ~0U && "Store within enum projection not handled");
1728+
auto &f = *theMemory->getFunction();
17201729
for (unsigned i : range(getNumSubElements(
1721-
address->getType().getObjectType(), mod,
1722-
TypeExpansionContext(*theMemory->getFunction())))) {
1730+
address->getType().getObjectType(), f,
1731+
TypeExpansionContext(f)))) {
17231732
// If this element is not required, don't fill it in.
17241733
if (!requiredElts[startSubElt + i])
17251734
continue;
@@ -1849,7 +1858,8 @@ void AvailableValueDataflowContext::updateAvailableValues(
18491858

18501859
bool AnyRequired = false;
18511860
for (unsigned i : range(getNumSubElements(
1852-
ValTy, getModule(), TypeExpansionContext(*CAI->getFunction())))) {
1861+
ValTy, getFunction(),
1862+
TypeExpansionContext(getFunction())))) {
18531863
// If this element is not required, don't fill it in.
18541864
AnyRequired = RequiredElts[StartSubElt+i];
18551865
if (AnyRequired) break;
@@ -1883,7 +1893,7 @@ void AvailableValueDataflowContext::updateAvailableValues(
18831893
// potentially bailing out (because it is address-only).
18841894
bool AnyRequired = false;
18851895
for (unsigned i : range(getNumSubElements(
1886-
ValTy, getModule(), TypeExpansionContext(*MD->getFunction())))) {
1896+
ValTy, getFunction(), TypeExpansionContext(getFunction())))) {
18871897
// If this element is not required, don't fill it in.
18881898
AnyRequired = RequiredElts[StartSubElt+i];
18891899
if (AnyRequired) break;
@@ -2171,7 +2181,7 @@ void AvailableValueDataflowContext::updateMarkDependenceValues(
21712181
}
21722182
SILType valueTy = md->getValue()->getType().getObjectType();
21732183
unsigned numMDSubElements = getNumSubElements(
2174-
valueTy, getModule(), TypeExpansionContext(*TheMemory->getFunction()));
2184+
valueTy, getFunction(), TypeExpansionContext(getFunction()));
21752185

21762186
// Update each required subelement of the mark_dependence value.
21772187
for (unsigned subIdx = firstMDElt; subIdx < firstMDElt + numMDSubElements;
@@ -2299,7 +2309,8 @@ class OptimizeAllocLoads {
22992309
: Module(memory->getModule()), TheMemory(memory),
23002310
MemoryType(getMemoryType(memory)),
23012311
NumMemorySubElements(getNumSubElements(
2302-
MemoryType, Module, TypeExpansionContext(*memory->getFunction()))),
2312+
MemoryType, *memory->getFunction(),
2313+
TypeExpansionContext(*memory->getFunction()))),
23032314
Uses(uses), deleter(deleter), deadEndBlocks(deadEndBlocks),
23042315
DataflowContext(TheMemory, NumMemorySubElements,
23052316
OptimizationMode::PreserveAlloc, uses,
@@ -2680,7 +2691,8 @@ class OptimizeDeadAlloc {
26802691
: Module(memory->getModule()), TheMemory(memory),
26812692
MemoryType(getMemoryType(memory)),
26822693
NumMemorySubElements(getNumSubElements(
2683-
MemoryType, Module, TypeExpansionContext(*memory->getFunction()))),
2694+
MemoryType, *memory->getFunction(),
2695+
TypeExpansionContext(*memory->getFunction()))),
26842696
Uses(uses), Releases(releases), deadEndBlocks(deadEndBlocks),
26852697
deleter(deleter), domInfo(domInfo),
26862698
DataflowContext(TheMemory, NumMemorySubElements,

‎test/SILOptimizer/predictable_deadalloc_elim_ownership.sil

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
// RUN: %target-sil-opt -sil-print-types -enable-sil-verify-all %s -predictable-deadalloc-elim | %FileCheck %s
1+
// RUN: %target-sil-opt -sil-print-types -enable-sil-verify-all %s -predictable-deadalloc-elim \
2+
// RUN: -enable-experimental-feature LifetimeDependence \
3+
// RUN: | %FileCheck %s
4+
5+
// REQUIRES: swift_feature_LifetimeDependence
26

37
sil_stage canonical
48

@@ -24,6 +28,8 @@ case some(T)
2428
protocol P {}
2529
struct S: P {}
2630

31+
struct NE: ~Escapable {}
32+
2733
///////////
2834
// Tests //
2935
///////////
@@ -762,3 +768,20 @@ bb0(%0 : $S):
762768
store %0 to [trivial] %4 : $*S
763769
unreachable
764770
}
771+
772+
// Preserve ownership for empty ~Escapable structs.
773+
//
774+
// CHECK-LABEL: sil hidden [ossa] @testEmptyNonEscapable : $@convention(method) (@guaranteed NE) -> @lifetime(copy 0) @owned NE {
775+
// CHECK: bb0(%0 : @guaranteed $NE):
776+
// CHECK-NEXT: %1 = copy_value %0 : $NE
777+
// CHECK-NEXT: return %1 : $NE
778+
// CHECK-LABEL: } // end sil function 'testEmptyNonEscapable'
779+
sil hidden [ossa] @testEmptyNonEscapable : $@convention(method) (@guaranteed NE) -> @lifetime(copy 0) @owned NE {
780+
bb0(%0 : @guaranteed $NE):
781+
%1 = copy_value %0
782+
%13 = alloc_stack $NE
783+
store %1 to [init] %13
784+
%22 = load [take] %13
785+
dealloc_stack %13
786+
return %22
787+
}

0 commit comments

Comments
 (0)