Skip to content

Commit 868dfd8

Browse files
committed
[SILOptimizer] Use BitfieldRef, not invalidate.
`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 868dfd8

File tree

2 files changed

+19
-21
lines changed

2 files changed

+19
-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: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,14 +1359,13 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
13591359

13601360
// Pruned liveness used to validate that load [take]/load [copy] can be
13611361
// converted to load_borrow without violating exclusivity.
1362-
SSAPrunedLiveness &liveness;
1362+
BitfieldRef<SSAPrunedLiveness> liveness;
13631363

13641364
GatherUsesVisitor(MoveOnlyAddressCheckerPImpl &moveChecker,
13651365
UseState &useState, MarkMustCheckInst *markedValue,
1366-
DiagnosticEmitter &diagnosticEmitter,
1367-
SSAPrunedLiveness &gatherUsesLiveness)
1366+
DiagnosticEmitter &diagnosticEmitter)
13681367
: moveChecker(moveChecker), useState(useState), markedValue(markedValue),
1369-
diagnosticEmitter(diagnosticEmitter), liveness(gatherUsesLiveness) {}
1368+
diagnosticEmitter(diagnosticEmitter) {}
13701369

13711370
bool visitUse(Operand *op) override;
13721371
void reset(MarkMustCheckInst *address) { useState.address = address; }
@@ -1383,7 +1382,8 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
13831382

13841383
/// Returns true if we emitted an error.
13851384
bool checkForExclusivityHazards(LoadInst *li) {
1386-
SWIFT_DEFER { liveness.invalidate(); };
1385+
BitfieldRef<SSAPrunedLiveness>::StackState state(liveness,
1386+
li->getFunction());
13871387

13881388
LLVM_DEBUG(llvm::dbgs() << "Checking for exclusivity hazards for: " << *li);
13891389

@@ -1404,10 +1404,10 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
14041404
}
14051405

14061406
bool emittedError = false;
1407-
liveness.initializeDef(bai);
1408-
liveness.computeSimple();
1407+
liveness->initializeDef(bai);
1408+
liveness->computeSimple();
14091409
for (auto *consumingUse : li->getConsumingUses()) {
1410-
if (!liveness.isWithinBoundary(consumingUse->getUser())) {
1410+
if (!liveness->isWithinBoundary(consumingUse->getUser())) {
14111411
diagnosticEmitter.emitAddressExclusivityHazardDiagnostic(
14121412
markedValue, consumingUse->getUser());
14131413
emittedError = true;
@@ -2415,10 +2415,8 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
24152415
// Then gather all uses of our address by walking from def->uses. We use this
24162416
// to categorize the uses of this address into their ownership behavior (e.x.:
24172417
// init, reinit, take, destroy, etc.).
2418-
SmallVector<SILBasicBlock *, 32> gatherUsesDiscoveredBlocks;
2419-
SSAPrunedLiveness gatherUsesLiveness(fn, &gatherUsesDiscoveredBlocks);
24202418
GatherUsesVisitor visitor(*this, addressUseState, markedAddress,
2421-
diagnosticEmitter, gatherUsesLiveness);
2419+
diagnosticEmitter);
24222420
SWIFT_DEFER { visitor.clear(); };
24232421
visitor.reset(markedAddress);
24242422
if (AddressUseKind::Unknown == std::move(visitor).walk(markedAddress)) {

0 commit comments

Comments
 (0)