Skip to content

Commit b3af244

Browse files
nate-chandlernate-chandler
authored
Merge pull request #84022 from nate-chandler/rdar158139991
[Mem2Reg] Add end_lifetime for more trivial case non-trivial enums.
2 parents f125703 + 1c52364 commit b3af244

File tree

2 files changed

+181
-49
lines changed

2 files changed

+181
-49
lines changed

‎lib/SILOptimizer/Transforms/SILMem2Reg.cpp‎

Lines changed: 61 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,7 @@ class LiveValues {
123123
}
124124

125125
void endLexicalLifetimeBeforeInst(AllocStackInst *asi,
126-
SILInstruction *beforeInstruction,
127-
SILBuilderContext &ctx);
126+
SILInstruction *beforeInstruction);
128127
};
129128
struct Guaranteed {
130129
SILValue stored = SILValue();
@@ -173,8 +172,7 @@ class LiveValues {
173172
}
174173

175174
void endLexicalLifetimeBeforeInst(AllocStackInst *asi,
176-
SILInstruction *beforeInstruction,
177-
SILBuilderContext &ctx);
175+
SILInstruction *beforeInstruction);
178176
};
179177
struct None {
180178
SILValue stored = SILValue();
@@ -190,8 +188,7 @@ class LiveValues {
190188
bool canEndLexicalLifetime() { return false; }
191189

192190
void endLexicalLifetimeBeforeInst(AllocStackInst *asi,
193-
SILInstruction *beforeInstruction,
194-
SILBuilderContext &ctx);
191+
SILInstruction *beforeInstruction);
195192
};
196193

197194
private:
@@ -289,23 +286,22 @@ class LiveValues {
289286
}
290287

291288
void endLexicalLifetimeBeforeInst(AllocStackInst *asi,
292-
SILInstruction *beforeInstruction,
293-
SILBuilderContext &ctx) {
289+
SILInstruction *beforeInstruction) {
294290
if (auto *owned = storage.dyn_cast<Owned>()) {
295-
return owned->endLexicalLifetimeBeforeInst(asi, beforeInstruction, ctx);
291+
return owned->endLexicalLifetimeBeforeInst(asi, beforeInstruction);
296292
} else if (auto *none = storage.dyn_cast<None>()) {
297-
return none->endLexicalLifetimeBeforeInst(asi, beforeInstruction, ctx);
293+
return none->endLexicalLifetimeBeforeInst(asi, beforeInstruction);
298294
}
299295
auto &guaranteed = storage.get<Guaranteed>();
300-
return guaranteed.endLexicalLifetimeBeforeInst(asi, beforeInstruction, ctx);
296+
return guaranteed.endLexicalLifetimeBeforeInst(asi, beforeInstruction);
301297
}
302298

303299
bool endLexicalLifetimeBeforeInstIfPossible(AllocStackInst *asi,
304300
SILInstruction *beforeInstruction,
305301
SILBuilderContext &ctx) {
306302
if (!canEndLexicalLifetime())
307303
return false;
308-
endLexicalLifetimeBeforeInst(asi, beforeInstruction, ctx);
304+
endLexicalLifetimeBeforeInst(asi, beforeInstruction);
309305
return true;
310306
}
311307
};
@@ -392,6 +388,17 @@ static bool isLoadFromStack(SILInstruction *i, AllocStackInst *asi) {
392388
return true;
393389
}
394390

391+
/// Whether the storage is invalid after \p i.
392+
///
393+
/// This is exactly when the instruction is a load [take].
394+
static bool doesLoadInvalidateStorage(SILInstruction *i) {
395+
auto *li = dyn_cast<LoadInst>(i);
396+
if (!li) {
397+
return false;
398+
}
399+
return li->getOwnershipQualifier() == LoadOwnershipQualifier::Take;
400+
}
401+
395402
/// Collects all load instructions which (transitively) use \p i as address.
396403
static void collectLoads(SILInstruction *i,
397404
SmallVectorImpl<SILInstruction *> &foundLoads) {
@@ -811,17 +818,15 @@ beginGuaranteedLexicalLifetimeAfterStore(AllocStackInst *asi,
811818
/// it will already have been ended naturally by destroy_addrs (or equivalent)
812819
/// of the alloc_stack.
813820
void LiveValues::Owned::endLexicalLifetimeBeforeInst(
814-
AllocStackInst *asi, SILInstruction *beforeInstruction,
815-
SILBuilderContext &ctx) {
821+
AllocStackInst *asi, SILInstruction *beforeInstruction) {
816822
assert(lexicalLifetimeEnsured(asi));
817823
assert(beforeInstruction);
818824
}
819825

820826
/// End the lexical borrow scope for an @guaranteed stored value described by
821827
/// the provided LiveValues struct before the specified instruction.
822828
void LiveValues::Guaranteed::endLexicalLifetimeBeforeInst(
823-
AllocStackInst *asi, SILInstruction *beforeInstruction,
824-
SILBuilderContext &ctx) {
829+
AllocStackInst *asi, SILInstruction *beforeInstruction) {
825830
assert(lexicalLifetimeEnsured(asi));
826831
assert(beforeInstruction);
827832
assert(borrow);
@@ -831,8 +836,7 @@ void LiveValues::Guaranteed::endLexicalLifetimeBeforeInst(
831836
}
832837

833838
void LiveValues::None::endLexicalLifetimeBeforeInst(
834-
AllocStackInst *asi, SILInstruction *beforeInstruction,
835-
SILBuilderContext &ctx) {
839+
AllocStackInst *asi, SILInstruction *beforeInstruction) {
836840
llvm::report_fatal_error(
837841
"can't have lexical lifetime for ownership none value");
838842
}
@@ -1061,7 +1065,7 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
10611065
if (runningVals && runningVals->value.canEndLexicalLifetime()) {
10621066
// End it right now if we have enough information.
10631067
runningVals->value.getOwned().endLexicalLifetimeBeforeInst(
1064-
asi, /*beforeInstruction=*/li, ctx);
1068+
asi, /*beforeInstruction=*/li);
10651069
} else {
10661070
// If we don't have enough information, end it endLexicalLifetime.
10671071
assert(!deinitializationPoints[blockPromotingWithin]);
@@ -1088,7 +1092,7 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
10881092
// StackAllocationPromoter::fixBranchesAndUses will later handle it.
10891093
LLVM_DEBUG(llvm::dbgs() << "*** First load: " << *li);
10901094
runningVals = {LiveValues::toReplace(asi, /*replacement=*/li),
1091-
/*isStorageValid=*/true};
1095+
/*isStorageValid=*/!doesLoadInvalidateStorage(inst)};
10921096
}
10931097
continue;
10941098
}
@@ -1143,7 +1147,7 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
11431147
if (oldRunningVals && oldRunningVals->isStorageValid &&
11441148
oldRunningVals->value.canEndLexicalLifetime()) {
11451149
oldRunningVals->value.getOwned().endLexicalLifetimeBeforeInst(
1146-
asi, /*beforeInstruction=*/si, ctx);
1150+
asi, /*beforeInstruction=*/si);
11471151
}
11481152
runningVals = beginOwnedLexicalLifetimeAfterStore(asi, si);
11491153
}
@@ -1175,9 +1179,6 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
11751179

11761180
// End the lexical lifetime of the store_borrow source.
11771181
if (auto *ebi = dyn_cast<EndBorrowInst>(inst)) {
1178-
if (!lexicalLifetimeEnsured(asi, lastStoreInst)) {
1179-
continue;
1180-
}
11811182
auto *sbi = dyn_cast<StoreBorrowInst>(ebi->getOperand());
11821183
if (!sbi) {
11831184
continue;
@@ -1196,8 +1197,10 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
11961197
if (sbi->getSrc() != runningVals->value.getStored()) {
11971198
continue;
11981199
}
1199-
// Mark storage as invalid and mark end_borrow as a deinit point.
12001200
runningVals->isStorageValid = false;
1201+
if (!lexicalLifetimeEnsured(asi, lastStoreInst)) {
1202+
continue;
1203+
}
12011204
runningVals->value.endLexicalLifetimeBeforeInstIfPossible(
12021205
asi, ebi->getNextInstruction(), ctx);
12031206
continue;
@@ -1214,6 +1217,7 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
12141217
continue;
12151218
}
12161219
if (runningVals) {
1220+
assert(runningVals->isStorageValid);
12171221
replaceDestroy(dai, runningVals->value.replacement(asi, dai), ctx,
12181222
deleter, instructionsToDelete);
12191223
if (lexicalLifetimeEnsured(asi, lastStoreInst)) {
@@ -1968,9 +1972,8 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
19681972
LiveValues::toReplace(asi,
19691973
/*replacement=*/createEmptyAndUndefValue(
19701974
asi->getElementType(), inst, ctx)),
1971-
/*isStorageValid=*/true};
1975+
/*isStorageValid=*/!doesLoadInvalidateStorage(inst)};
19721976
}
1973-
assert(runningVals && runningVals->isStorageValid);
19741977
auto *loadInst = dyn_cast<LoadInst>(inst);
19751978
if (loadInst &&
19761979
loadInst->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
@@ -2039,13 +2042,13 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
20392042
if (!runningVals.has_value()) {
20402043
continue;
20412044
}
2042-
if (!runningVals->value.isGuaranteed()) {
2043-
continue;
2044-
}
20452045
if (sbi->getSrc() != runningVals->value.getStored()) {
20462046
continue;
20472047
}
20482048
runningVals->isStorageValid = false;
2049+
if (!runningVals->value.isGuaranteed()) {
2050+
continue;
2051+
}
20492052
runningVals->value.endLexicalLifetimeBeforeInstIfPossible(
20502053
asi, ebi->getNextInstruction(), ctx);
20512054
continue;
@@ -2094,24 +2097,32 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
20942097
}
20952098
}
20962099

2097-
if (lexicalLifetimeEnsured(asi) && runningVals &&
2098-
runningVals->isStorageValid &&
2099-
runningVals->value.getStored()->getOwnershipKind().isCompatibleWith(
2100-
OwnershipKind::Owned)) {
2101-
// There is still valid storage after visiting all instructions in this
2102-
// block which are the only instructions involving this alloc_stack.
2103-
// This can only happen if all paths from this block end in unreachable.
2104-
//
2105-
// We need to end the lexical lifetime at the last possible location, at the
2106-
// boundary blocks which are the predecessors of dominance frontier
2107-
// dominated by the alloc_stack.
2108-
SmallVector<SILBasicBlock *, 4> boundary;
2109-
computeDominatedBoundaryBlocks(asi->getParent(), domInfo, boundary);
2110-
for (auto *block : boundary) {
2111-
auto *terminator = block->getTerminator();
2112-
runningVals->value.endLexicalLifetimeBeforeInstIfPossible(
2113-
asi, /*beforeInstruction=*/terminator, ctx);
2114-
}
2100+
auto *function = asi->getFunction();
2101+
if (!function->hasOwnership() || !runningVals ||
2102+
!runningVals->isStorageValid ||
2103+
asi->getElementType().isTrivial(function)) {
2104+
return;
2105+
}
2106+
// There is still valid storage after visiting all instructions in this
2107+
// block which are the only instructions involving this alloc_stack.
2108+
// That can happen if:
2109+
// (1) this block is a dead-end. TODO: OSSACompleteLifetime: Complete such
2110+
// lifetimes.
2111+
// (2) a trivial case of a non-trivial enum was stored to the address
2112+
2113+
auto *deadEndBlocks = deadEndBlocksAnalysis->get(function);
2114+
2115+
if (!deadEndBlocks->isDeadEnd(parentBlock)) {
2116+
// We may have incomplete lifetimes for enum locations on trivial paths.
2117+
// After promoting them, complete lifetime here.
2118+
ASSERT(asi->getElementType().isOrHasEnum());
2119+
OSSACompleteLifetime completion(function, domInfo, *deadEndBlocks,
2120+
OSSACompleteLifetime::IgnoreTrivialVariable,
2121+
/*forceLivenessVerification=*/false,
2122+
/*nonDestroyingEnd=*/true);
2123+
completion.completeOSSALifetime(
2124+
runningVals->value.replacement(asi, nullptr),
2125+
OSSACompleteLifetime::Boundary::Liveness);
21152126
}
21162127
}
21172128

@@ -2211,7 +2222,8 @@ bool MemoryToRegisters::promoteAllocation(AllocStackInst *alloc,
22112222
}
22122223

22132224
// Remove write-only AllocStacks.
2214-
if (isWriteOnlyAllocation(alloc) && !lexicalLifetimeEnsured(alloc)) {
2225+
if (isWriteOnlyAllocation(alloc) && !alloc->getType().isOrHasEnum() &&
2226+
!lexicalLifetimeEnsured(alloc)) {
22152227
LLVM_DEBUG(llvm::dbgs() << "*** Deleting store-only AllocStack: "<< *alloc);
22162228
deleter.forceDeleteWithUsers(alloc);
22172229
return true;

‎test/SILOptimizer/mem2reg_ossa_nontrivial.sil‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,3 +1591,123 @@ bb3:
15911591
return %r
15921592
}
15931593

1594+
// CHECK-LABEL: sil [ossa] @end_lifetime_at_trivial_case_dealloc_stack : {{.*}} {
1595+
// CHECK-NOT: alloc_stack
1596+
// CHECK: [[IN:%[^,]+]] = load [copy]
1597+
// CHECK: [[IN_BORROW:%[^,]+]] = begin_borrow [[IN]]
1598+
// CHECK: switch_enum [[IN_BORROW]]
1599+
// CHECK-SAME: case #Result.success!enumelt: [[INT:bb[0-9]+]]
1600+
// CHECK: [[INT]]({{%[^,]+}} : $Int):
1601+
// CHECK: end_borrow [[IN_BORROW]]
1602+
// CHECK: end_lifetime [[IN]]
1603+
// CHECK-LABEL: } // end sil function 'end_lifetime_at_trivial_case_dealloc_stack'
1604+
sil [ossa] @end_lifetime_at_trivial_case_dealloc_stack : $@convention(thin) (@in_guaranteed Result<Int, X>) -> () {
1605+
entry(%in_addr : $*Result<Int, X>):
1606+
%main_addr = alloc_stack $Result<Int, X>
1607+
%in = load [copy] %in_addr
1608+
store %in to [init] %main_addr
1609+
%in_borrow = load_borrow %main_addr
1610+
switch_enum %in_borrow,
1611+
case #Result.success!enumelt: int,
1612+
case #Result.failure!enumelt: full
1613+
1614+
int(%6 : $Int):
1615+
end_borrow %in_borrow
1616+
%temp = alloc_stack $Result<Int, X>
1617+
%in_copy = load [copy] %main_addr
1618+
store %in_copy to [init] %temp
1619+
br int2
1620+
1621+
int2:
1622+
dealloc_stack %temp
1623+
br exit
1624+
1625+
full(%13 : @guaranteed $X):
1626+
end_borrow %in_borrow
1627+
br exit
1628+
1629+
exit:
1630+
destroy_addr %main_addr
1631+
dealloc_stack %main_addr
1632+
%18 = tuple ()
1633+
return %18
1634+
}
1635+
1636+
// CHECK-LABEL: sil [ossa] @end_lifetime_at_trivial_case_dealloc_stack_2 : {{.*}} {
1637+
// CHECK: bb0([[IN_ADDR:%[^,]+]] :
1638+
// CHECK: [[IN_COPY:%[^,]+]] = load [copy] [[IN_ADDR]]
1639+
// CHECK: [[IN_BORROW:%[^,]+]] = begin_borrow [[IN_COPY]]
1640+
// CHECK: switch_enum [[IN_BORROW]]
1641+
// CHECK: case #Result.success!enumelt: [[INT:bb[0-9]+]]
1642+
// CHECK: [[INT]]
1643+
// CHECK: end_borrow [[IN_BORROW]]
1644+
// CHECK: end_lifetime [[IN_COPY]]
1645+
// CHECK-LABEL: } // end sil function 'end_lifetime_at_trivial_case_dealloc_stack_2'
1646+
sil [ossa] @end_lifetime_at_trivial_case_dealloc_stack_2 : $@convention(thin) (@in_guaranteed Result<Int, X>) -> () {
1647+
entry(%in_addr : $*Result<Int, X>):
1648+
%main_addr = alloc_stack $Result<Int, X>
1649+
%temp = alloc_stack $Result<Int, X>
1650+
%in = load [copy] %in_addr
1651+
store %in to [init] %main_addr
1652+
%in_borrow = load_borrow %main_addr
1653+
switch_enum %in_borrow,
1654+
case #Result.success!enumelt: int,
1655+
case #Result.failure!enumelt: x
1656+
1657+
int(%int : $Int):
1658+
end_borrow %in_borrow
1659+
%in_copy = load [copy] %main_addr
1660+
store %in_copy to [init] %temp
1661+
br int2
1662+
1663+
int2:
1664+
br exit
1665+
1666+
x(%13 : @guaranteed $X):
1667+
end_borrow %in_borrow
1668+
br exit
1669+
1670+
exit:
1671+
destroy_addr %main_addr
1672+
dealloc_stack %temp
1673+
dealloc_stack %main_addr
1674+
%retval = tuple ()
1675+
return %retval
1676+
}
1677+
1678+
// CHECK-LABEL: sil [ossa] @end_lifetime_at_trivial_case_dealloc_stack_3 : {{.*}} {
1679+
// CHECK: bb0([[IN_ADDR:%[^,]+]] :
1680+
// CHECK: [[IN_BORROW:%[^,]+]] = load_borrow [[IN_ADDR]]
1681+
// CHECK: switch_enum [[IN_BORROW]]
1682+
// CHECK: case #Result.success!enumelt: [[INT:bb[0-9]+]]
1683+
// CHECK: [[INT]]
1684+
// CHECK: end_borrow [[IN_BORROW]]
1685+
// CHECK: [[IN_COPY:%[^,]+]] = load [copy] [[IN_ADDR]]
1686+
// CHECK: end_lifetime [[IN_COPY]]
1687+
// CHECK-LABEL: } // end sil function 'end_lifetime_at_trivial_case_dealloc_stack_3'
1688+
sil [ossa] @end_lifetime_at_trivial_case_dealloc_stack_3 : $@convention(thin) (@in_guaranteed Result<Int, X>) -> () {
1689+
entry(%in_addr : $*Result<Int, X>):
1690+
%in_borrow = load_borrow %in_addr
1691+
switch_enum %in_borrow,
1692+
case #Result.success!enumelt: int,
1693+
case #Result.failure!enumelt: x
1694+
1695+
int(%int : $Int):
1696+
end_borrow %in_borrow
1697+
%temp = alloc_stack $Result<Int, X>
1698+
%in_copy = load [copy] %in_addr
1699+
store %in_copy to [init] %temp
1700+
dealloc_stack %temp
1701+
br int2
1702+
1703+
int2:
1704+
br exit
1705+
1706+
x(%13 : @guaranteed $X):
1707+
end_borrow %in_borrow
1708+
br exit
1709+
1710+
exit:
1711+
%retval = tuple ()
1712+
return %retval
1713+
}

0 commit comments

Comments
 (0)