Skip to content

Commit 7d8198e

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 0897748 commit 7d8198e

File tree

2 files changed

+25
-22
lines changed

2 files changed

+25
-22
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: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,14 +1359,17 @@ 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

1364-
GatherUsesVisitor(MoveOnlyAddressCheckerPImpl &moveChecker,
1365-
UseState &useState, MarkMustCheckInst *markedValue,
1366-
DiagnosticEmitter &diagnosticEmitter,
1367-
SSAPrunedLiveness &gatherUsesLiveness)
1364+
SmallVectorImpl<SILBasicBlock *> &gatherUsesDiscoveredBlocks;
1365+
1366+
GatherUsesVisitor(
1367+
MoveOnlyAddressCheckerPImpl &moveChecker, UseState &useState,
1368+
MarkMustCheckInst *markedValue, DiagnosticEmitter &diagnosticEmitter,
1369+
SmallVectorImpl<SILBasicBlock *> &gatherUsesDiscoveredBlocks)
13681370
: moveChecker(moveChecker), useState(useState), markedValue(markedValue),
1369-
diagnosticEmitter(diagnosticEmitter), liveness(gatherUsesLiveness) {}
1371+
diagnosticEmitter(diagnosticEmitter),
1372+
gatherUsesDiscoveredBlocks(gatherUsesDiscoveredBlocks) {}
13701373

13711374
bool visitUse(Operand *op) override;
13721375
void reset(MarkMustCheckInst *address) { useState.address = address; }
@@ -1383,7 +1386,8 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
13831386

13841387
/// Returns true if we emitted an error.
13851388
bool checkForExclusivityHazards(LoadInst *li) {
1386-
SWIFT_DEFER { liveness.invalidate(); };
1389+
BitfieldRef<SSAPrunedLiveness>::StackState state(
1390+
liveness, li->getFunction(), &gatherUsesDiscoveredBlocks);
13871391

13881392
LLVM_DEBUG(llvm::dbgs() << "Checking for exclusivity hazards for: " << *li);
13891393

@@ -1404,10 +1408,10 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
14041408
}
14051409

14061410
bool emittedError = false;
1407-
liveness.initializeDef(bai);
1408-
liveness.computeSimple();
1411+
liveness->initializeDef(bai);
1412+
liveness->computeSimple();
14091413
for (auto *consumingUse : li->getConsumingUses()) {
1410-
if (!liveness.isWithinBoundary(consumingUse->getUser())) {
1414+
if (!liveness->isWithinBoundary(consumingUse->getUser())) {
14111415
diagnosticEmitter.emitAddressExclusivityHazardDiagnostic(
14121416
markedValue, consumingUse->getUser());
14131417
emittedError = true;
@@ -2416,9 +2420,8 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
24162420
// to categorize the uses of this address into their ownership behavior (e.x.:
24172421
// init, reinit, take, destroy, etc.).
24182422
SmallVector<SILBasicBlock *, 32> gatherUsesDiscoveredBlocks;
2419-
SSAPrunedLiveness gatherUsesLiveness(fn, &gatherUsesDiscoveredBlocks);
24202423
GatherUsesVisitor visitor(*this, addressUseState, markedAddress,
2421-
diagnosticEmitter, gatherUsesLiveness);
2424+
diagnosticEmitter, gatherUsesDiscoveredBlocks);
24222425
SWIFT_DEFER { visitor.clear(); };
24232426
visitor.reset(markedAddress);
24242427
if (AddressUseKind::Unknown == std::move(visitor).walk(markedAddress)) {

0 commit comments

Comments
 (0)