Skip to content

Commit b530531

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 e32b621 commit b530531

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
@@ -1374,16 +1374,19 @@ struct GatherUsesVisitor : public AccessUseVisitor {
13741374

13751375
// Pruned liveness used to validate that load [take]/load [copy] can be
13761376
// converted to load_borrow without violating exclusivity.
1377-
SSAPrunedLiveness &liveness;
1377+
BitfieldRef<SSAPrunedLiveness> liveness;
13781378

1379-
GatherUsesVisitor(MoveOnlyAddressCheckerPImpl &moveChecker,
1380-
UseState &useState, MarkMustCheckInst *markedValue,
1381-
DiagnosticEmitter &diagnosticEmitter,
1382-
SSAPrunedLiveness &gatherUsesLiveness)
1379+
SmallVectorImpl<SILBasicBlock *> &gatherUsesDiscoveredBlocks;
1380+
1381+
GatherUsesVisitor(
1382+
MoveOnlyAddressCheckerPImpl &moveChecker, UseState &useState,
1383+
MarkMustCheckInst *markedValue, DiagnosticEmitter &diagnosticEmitter,
1384+
SmallVectorImpl<SILBasicBlock *> &gatherUsesDiscoveredBlocks)
13831385
: AccessUseVisitor(AccessUseType::Inner,
13841386
NestedAccessType::IgnoreAccessBegin),
13851387
moveChecker(moveChecker), useState(useState), markedValue(markedValue),
1386-
diagnosticEmitter(diagnosticEmitter), liveness(gatherUsesLiveness) {}
1388+
diagnosticEmitter(diagnosticEmitter),
1389+
gatherUsesDiscoveredBlocks(gatherUsesDiscoveredBlocks) {}
13871390

13881391
bool visitUse(Operand *op, AccessUseType useTy) override;
13891392
void reset(MarkMustCheckInst *address) { useState.address = address; }
@@ -1400,7 +1403,8 @@ struct GatherUsesVisitor : public AccessUseVisitor {
14001403

14011404
/// Returns true if we emitted an error.
14021405
bool checkForExclusivityHazards(LoadInst *li) {
1403-
SWIFT_DEFER { liveness.invalidate(); };
1406+
BitfieldRef<SSAPrunedLiveness>::StackState state(
1407+
liveness, li->getFunction(), &gatherUsesDiscoveredBlocks);
14041408

14051409
LLVM_DEBUG(llvm::dbgs() << "Checking for exclusivity hazards for: " << *li);
14061410

@@ -1421,10 +1425,10 @@ struct GatherUsesVisitor : public AccessUseVisitor {
14211425
}
14221426

14231427
bool emittedError = false;
1424-
liveness.initializeDef(bai);
1425-
liveness.computeSimple();
1428+
liveness->initializeDef(bai);
1429+
liveness->computeSimple();
14261430
for (auto *consumingUse : li->getConsumingUses()) {
1427-
if (!liveness.isWithinBoundary(consumingUse->getUser())) {
1431+
if (!liveness->isWithinBoundary(consumingUse->getUser())) {
14281432
diagnosticEmitter.emitAddressExclusivityHazardDiagnostic(
14291433
markedValue, consumingUse->getUser());
14301434
emittedError = true;
@@ -2452,9 +2456,8 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
24522456
// to categorize the uses of this address into their ownership behavior (e.x.:
24532457
// init, reinit, take, destroy, etc.).
24542458
SmallVector<SILBasicBlock *, 32> gatherUsesDiscoveredBlocks;
2455-
SSAPrunedLiveness gatherUsesLiveness(fn, &gatherUsesDiscoveredBlocks);
24562459
GatherUsesVisitor visitor(*this, addressUseState, markedAddress,
2457-
diagnosticEmitter, gatherUsesLiveness);
2460+
diagnosticEmitter, gatherUsesDiscoveredBlocks);
24582461
SWIFT_DEFER { visitor.clear(); };
24592462
visitor.reset(markedAddress);
24602463
if (!visitAccessPathBaseUses(visitor, accessPathWithBase, fn)) {

0 commit comments

Comments
 (0)