Skip to content

Commit 9cb5593

Browse files
committed
[SILOptimizer] Used local SSAPrunedLivenesses.
`SSAPrunedLiveness::invalidate` is used as though it reset the state of the instance it is called on. Clients then reuse the instance with the expectation that it has been reset. But since it has not been reset, this results in unexpected liveness results. rdar://108627366
1 parent 2c08dc2 commit 9cb5593

File tree

2 files changed

+16
-21
lines changed

2 files changed

+16
-21
lines changed

lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class DiagnoseLifetimeIssues {
6666
SILFunction *function = nullptr;
6767

6868
/// The liveness of the object in question, computed in visitUses.
69-
SSAPrunedLiveness liveness;
69+
BitfieldRef<SSAPrunedLiveness> liveness;
7070

7171
/// All weak stores of the object, which are found in visitUses.
7272
llvm::SmallVector<SILInstruction *, 8> weakStores;
@@ -86,8 +86,7 @@ class DiagnoseLifetimeIssues {
8686
void reportDeadStore(SILInstruction *allocationInst);
8787

8888
public:
89-
DiagnoseLifetimeIssues(SILFunction *function)
90-
: function(function), liveness(function) {}
89+
DiagnoseLifetimeIssues(SILFunction *function) : function(function) {}
9190

9291
void diagnose();
9392
};
@@ -215,7 +214,7 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
215214
case OperandOwnership::UnownedInstantaneousUse:
216215
case OperandOwnership::BitwiseEscape:
217216
if (updateLivenessAndWeakStores)
218-
liveness.updateForUse(user, /*lifetimeEnding*/ false);
217+
liveness->updateForUse(user, /*lifetimeEnding*/ false);
219218
break;
220219
case OperandOwnership::GuaranteedForwarding:
221220
case OperandOwnership::ForwardingConsume:
@@ -241,9 +240,9 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
241240
return CanEscape;
242241
break;
243242
case OperandOwnership::Borrow: {
244-
if (updateLivenessAndWeakStores
245-
&& (liveness.updateForBorrowingOperand(use)
246-
!= InnerBorrowKind::Contained)) {
243+
if (updateLivenessAndWeakStores &&
244+
(liveness->updateForBorrowingOperand(use) !=
245+
InnerBorrowKind::Contained)) {
247246
return CanEscape;
248247
}
249248
BorrowingOperand borrowOper(use);
@@ -326,11 +325,12 @@ static bool isOutOfLifetime(SILInstruction *inst, SSAPrunedLiveness &liveness) {
326325
/// Reports a warning if the stored object \p storedObj is never loaded within
327326
/// the lifetime of the stored object.
328327
void DiagnoseLifetimeIssues::reportDeadStore(SILInstruction *allocationInst) {
329-
liveness.invalidate();
328+
BitfieldRef<SSAPrunedLiveness>::StackState livenessBitfieldContainer(
329+
liveness, allocationInst->getFunction());
330330
weakStores.clear();
331331

332332
SILValue storedDef = cast<SingleValueInstruction>(allocationInst);
333-
liveness.initializeDef(storedDef);
333+
liveness->initializeDef(storedDef);
334334

335335
// Compute the canonical lifetime of storedDef, like the copy-propagation pass
336336
// would do.
@@ -346,7 +346,7 @@ void DiagnoseLifetimeIssues::reportDeadStore(SILInstruction *allocationInst) {
346346
assert((state == IsStoredWeakly) == !weakStores.empty());
347347

348348
for (SILInstruction *storeInst : weakStores) {
349-
if (isOutOfLifetime(storeInst, liveness)) {
349+
if (isOutOfLifetime(storeInst, *liveness)) {
350350
// Issue the warning.
351351
storeInst->getModule().getASTContext().Diags.diagnose(
352352
storeInst->getLoc().getSourceLoc(), diag::warn_dead_weak_store);

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,16 +1357,11 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
13571357
bool emittedEarlyDiagnostic = false;
13581358
DiagnosticEmitter &diagnosticEmitter;
13591359

1360-
// Pruned liveness used to validate that load [take]/load [copy] can be
1361-
// converted to load_borrow without violating exclusivity.
1362-
SSAPrunedLiveness &liveness;
1363-
13641360
GatherUsesVisitor(MoveOnlyAddressCheckerPImpl &moveChecker,
13651361
UseState &useState, MarkMustCheckInst *markedValue,
1366-
DiagnosticEmitter &diagnosticEmitter,
1367-
SSAPrunedLiveness &gatherUsesLiveness)
1362+
DiagnosticEmitter &diagnosticEmitter)
13681363
: moveChecker(moveChecker), useState(useState), markedValue(markedValue),
1369-
diagnosticEmitter(diagnosticEmitter), liveness(gatherUsesLiveness) {}
1364+
diagnosticEmitter(diagnosticEmitter) {}
13701365

13711366
bool visitUse(Operand *op) override;
13721367
void reset(MarkMustCheckInst *address) { useState.address = address; }
@@ -1383,7 +1378,9 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
13831378

13841379
/// Returns true if we emitted an error.
13851380
bool checkForExclusivityHazards(LoadInst *li) {
1386-
SWIFT_DEFER { liveness.invalidate(); };
1381+
// Pruned liveness used to validate that load [take]/load [copy] can be
1382+
// converted to load_borrow without violating exclusivity.
1383+
SSAPrunedLiveness liveness(li->getFunction());
13871384

13881385
LLVM_DEBUG(llvm::dbgs() << "Checking for exclusivity hazards for: " << *li);
13891386

@@ -2415,10 +2412,8 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
24152412
// Then gather all uses of our address by walking from def->uses. We use this
24162413
// to categorize the uses of this address into their ownership behavior (e.x.:
24172414
// init, reinit, take, destroy, etc.).
2418-
SmallVector<SILBasicBlock *, 32> gatherUsesDiscoveredBlocks;
2419-
SSAPrunedLiveness gatherUsesLiveness(fn, &gatherUsesDiscoveredBlocks);
24202415
GatherUsesVisitor visitor(*this, addressUseState, markedAddress,
2421-
diagnosticEmitter, gatherUsesLiveness);
2416+
diagnosticEmitter);
24222417
SWIFT_DEFER { visitor.clear(); };
24232418
visitor.reset(markedAddress);
24242419
if (AddressUseKind::Unknown == std::move(visitor).walk(markedAddress)) {

0 commit comments

Comments
 (0)