Skip to content

Commit 63a3c65

Browse files
committed
[Mem2Reg] Fix isStorageValid.
In a few places, it wasn't getting set to false: `end_borrow`s of `store_borrow`s and `load [take]`s.
1 parent 09b1c31 commit 63a3c65

File tree

1 file changed

+20
-10
lines changed

1 file changed

+20
-10
lines changed

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,17 @@ static bool isLoadFromStack(SILInstruction *i, AllocStackInst *asi) {
388388
return true;
389389
}
390390

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+
391402
/// Collects all load instructions which (transitively) use \p i as address.
392403
static void collectLoads(SILInstruction *i,
393404
SmallVectorImpl<SILInstruction *> &foundLoads) {
@@ -1081,7 +1092,7 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
10811092
// StackAllocationPromoter::fixBranchesAndUses will later handle it.
10821093
LLVM_DEBUG(llvm::dbgs() << "*** First load: " << *li);
10831094
runningVals = {LiveValues::toReplace(asi, /*replacement=*/li),
1084-
/*isStorageValid=*/true};
1095+
/*isStorageValid=*/!doesLoadInvalidateStorage(inst)};
10851096
}
10861097
continue;
10871098
}
@@ -1168,9 +1179,6 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
11681179

11691180
// End the lexical lifetime of the store_borrow source.
11701181
if (auto *ebi = dyn_cast<EndBorrowInst>(inst)) {
1171-
if (!lexicalLifetimeEnsured(asi, lastStoreInst)) {
1172-
continue;
1173-
}
11741182
auto *sbi = dyn_cast<StoreBorrowInst>(ebi->getOperand());
11751183
if (!sbi) {
11761184
continue;
@@ -1189,8 +1197,10 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
11891197
if (sbi->getSrc() != runningVals->value.getStored()) {
11901198
continue;
11911199
}
1192-
// Mark storage as invalid and mark end_borrow as a deinit point.
11931200
runningVals->isStorageValid = false;
1201+
if (!lexicalLifetimeEnsured(asi, lastStoreInst)) {
1202+
continue;
1203+
}
11941204
runningVals->value.endLexicalLifetimeBeforeInstIfPossible(
11951205
asi, ebi->getNextInstruction(), ctx);
11961206
continue;
@@ -1207,6 +1217,7 @@ SILInstruction *StackAllocationPromoter::promoteAllocationInBlock(
12071217
continue;
12081218
}
12091219
if (runningVals) {
1220+
assert(runningVals->isStorageValid);
12101221
replaceDestroy(dai, runningVals->value.replacement(asi, dai), ctx,
12111222
deleter, instructionsToDelete);
12121223
if (lexicalLifetimeEnsured(asi, lastStoreInst)) {
@@ -1961,9 +1972,8 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
19611972
LiveValues::toReplace(asi,
19621973
/*replacement=*/createEmptyAndUndefValue(
19631974
asi->getElementType(), inst, ctx)),
1964-
/*isStorageValid=*/true};
1975+
/*isStorageValid=*/!doesLoadInvalidateStorage(inst)};
19651976
}
1966-
assert(runningVals && runningVals->isStorageValid);
19671977
auto *loadInst = dyn_cast<LoadInst>(inst);
19681978
if (loadInst &&
19691979
loadInst->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
@@ -2032,13 +2042,13 @@ void MemoryToRegisters::removeSingleBlockAllocation(AllocStackInst *asi) {
20322042
if (!runningVals.has_value()) {
20332043
continue;
20342044
}
2035-
if (!runningVals->value.isGuaranteed()) {
2036-
continue;
2037-
}
20382045
if (sbi->getSrc() != runningVals->value.getStored()) {
20392046
continue;
20402047
}
20412048
runningVals->isStorageValid = false;
2049+
if (!runningVals->value.isGuaranteed()) {
2050+
continue;
2051+
}
20422052
runningVals->value.endLexicalLifetimeBeforeInstIfPossible(
20432053
asi, ebi->getNextInstruction(), ctx);
20442054
continue;

0 commit comments

Comments
 (0)