Skip to content

Commit 1691b37

Browse files
Merge pull request #65490 from nate-chandler/cherrypick/release/5.9/rdar108627366
5.9: [SILOptimizer] Use BitfieldRef instead of invalidating SSAPrunedLiveness.
2 parents 0670d6f + 5c6cefe commit 1691b37

File tree

4 files changed

+43
-43
lines changed

4 files changed

+43
-43
lines changed

include/swift/SIL/SILBitfield.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,9 @@ template <typename BitfieldContainer> struct BitfieldRef {
186186
BitfieldRef &ref;
187187
BitfieldContainer container;
188188

189-
StackState(BitfieldRef &ref, SILFunction *function)
190-
: ref(ref), container(function) {
189+
template <typename... ArgTypes>
190+
StackState(BitfieldRef &ref, ArgTypes &&...Args)
191+
: ref(ref), container(std::forward<ArgTypes>(Args)...) {
191192
ref.ref = &container;
192193
}
193194

lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,12 @@ struct CheckerLivenessInfo {
5858
SmallSetVector<Operand *, 8> consumingUse;
5959
SmallSetVector<SILInstruction *, 8> nonLifetimeEndingUsesInLiveOut;
6060
SmallVector<Operand *, 8> interiorPointerTransitiveUses;
61-
DiagnosticPrunedLiveness liveness;
61+
BitfieldRef<DiagnosticPrunedLiveness> liveness;
6262

63-
CheckerLivenessInfo(SILFunction *fn)
64-
: nonLifetimeEndingUsesInLiveOut(),
65-
liveness(fn, nullptr, &nonLifetimeEndingUsesInLiveOut) {}
63+
CheckerLivenessInfo() : nonLifetimeEndingUsesInLiveOut() {}
6664

6765
void initDef(SILValue def) {
68-
liveness.initializeDef(def);
66+
liveness->initializeDef(def);
6967
defUseWorklist.insert(def);
7068
}
7169

@@ -78,7 +76,6 @@ struct CheckerLivenessInfo {
7876

7977
void clear() {
8078
defUseWorklist.clear();
81-
liveness.invalidate();
8279
consumingUse.clear();
8380
interiorPointerTransitiveUses.clear();
8481
nonLifetimeEndingUsesInLiveOut.clear();
@@ -121,16 +118,16 @@ bool CheckerLivenessInfo::compute() {
121118
case OperandOwnership::InstantaneousUse:
122119
case OperandOwnership::UnownedInstantaneousUse:
123120
case OperandOwnership::BitwiseEscape:
124-
liveness.updateForUse(user, /*lifetimeEnding*/ false);
121+
liveness->updateForUse(user, /*lifetimeEnding*/ false);
125122
break;
126123
case OperandOwnership::ForwardingConsume:
127124
consumingUse.insert(use);
128-
liveness.updateForUse(user, /*lifetimeEnding*/ true);
125+
liveness->updateForUse(user, /*lifetimeEnding*/ true);
129126
break;
130127
case OperandOwnership::DestroyingConsume:
131128
// destroy_value does not force pruned liveness (but store etc. does).
132129
if (!isa<DestroyValueInst>(user)) {
133-
liveness.updateForUse(user, /*lifetimeEnding*/ true);
130+
liveness->updateForUse(user, /*lifetimeEnding*/ true);
134131
}
135132
consumingUse.insert(use);
136133
break;
@@ -147,12 +144,12 @@ bool CheckerLivenessInfo::compute() {
147144
// of the new variable as a use. Thus we only include the begin_borrow
148145
// itself as the use.
149146
if (bbi->isLexical()) {
150-
liveness.updateForUse(bbi, false /*lifetime ending*/);
147+
liveness->updateForUse(bbi, false /*lifetime ending*/);
151148
} else {
152149
// Otherwise, try to update liveness for a borrowing operand
153150
// use. This will make it so that we add the end_borrows of the
154151
// liveness use. If we have a reborrow here, we will bail.
155-
if (liveness.updateForBorrowingOperand(use) !=
152+
if (liveness->updateForBorrowingOperand(use) !=
156153
InnerBorrowKind::Contained) {
157154
return false;
158155
}
@@ -163,7 +160,7 @@ bool CheckerLivenessInfo::compute() {
163160
case OperandOwnership::GuaranteedForwarding:
164161
// A forwarding borrow is validated as part of its parent borrow. So
165162
// just mark it as extending liveness and look through it.
166-
liveness.updateForUse(user, /*lifetimeEnding*/ false);
163+
liveness->updateForUse(user, /*lifetimeEnding*/ false);
167164
ForwardingOperand(use).visitForwardedValues([&](SILValue result) {
168165
if (SILArgument::isTerminatorResult(result)) {
169166
return true;
@@ -187,7 +184,8 @@ bool CheckerLivenessInfo::compute() {
187184
(void)addrUseKind;
188185
while (!interiorPointerTransitiveUses.empty()) {
189186
auto *addrUse = interiorPointerTransitiveUses.pop_back_val();
190-
liveness.updateForUse(addrUse->getUser(), /*lifetimeEnding*/ false);
187+
liveness->updateForUse(addrUse->getUser(),
188+
/*lifetimeEnding*/ false);
191189
}
192190
}
193191
break;
@@ -220,8 +218,7 @@ struct ConsumeOperatorCopyableValuesChecker {
220218
SILLoopInfo *loopInfoToUpdate;
221219

222220
ConsumeOperatorCopyableValuesChecker(SILFunction *fn)
223-
: fn(fn), livenessInfo(fn), dominanceToUpdate(nullptr),
224-
loopInfoToUpdate(nullptr) {}
221+
: fn(fn), dominanceToUpdate(nullptr), loopInfoToUpdate(nullptr) {}
225222

226223
void setDominanceToUpdate(DominanceInfo *newDFI) {
227224
dominanceToUpdate = newDFI;
@@ -262,7 +259,7 @@ void ConsumeOperatorCopyableValuesChecker::emitDiagnosticForMove(
262259
// going to be emitting a diagnostic and thus later parts of the compiler are
263260
// not going to run. First we look for uses in the same block as our move.
264261
auto *mviBlock = mvi->getParent();
265-
auto mviBlockLiveness = livenessInfo.liveness.getBlockLiveness(mviBlock);
262+
auto mviBlockLiveness = livenessInfo.liveness->getBlockLiveness(mviBlock);
266263
switch (mviBlockLiveness) {
267264
case PrunedLiveBlocks::Dead:
268265
llvm_unreachable("We should never see this");
@@ -278,7 +275,7 @@ void ConsumeOperatorCopyableValuesChecker::emitDiagnosticForMove(
278275
// implementation choice.
279276
for (SILInstruction &inst :
280277
make_range(std::next(mvi->getIterator()), mviBlock->end())) {
281-
switch (livenessInfo.liveness.isInterestingUser(&inst)) {
278+
switch (livenessInfo.liveness->isInterestingUser(&inst)) {
282279
case PrunedLiveness::NonUser:
283280
break;
284281
case PrunedLiveness::NonLifetimeEndingUse:
@@ -334,9 +331,9 @@ void ConsumeOperatorCopyableValuesChecker::emitDiagnosticForMove(
334331
// adding successors since we do not need to look further than the pruned
335332
// liveness boundary for uses.
336333
if (PrunedLiveBlocks::LiveOut !=
337-
livenessInfo.liveness.getBlockLiveness(block)) {
334+
livenessInfo.liveness->getBlockLiveness(block)) {
338335
for (SILInstruction &inst : *block) {
339-
switch (livenessInfo.liveness.isInterestingUser(&inst)) {
336+
switch (livenessInfo.liveness->isInterestingUser(&inst)) {
340337
case PrunedLiveness::NonUser:
341338
break;
342339
case PrunedLiveness::NonLifetimeEndingUse:
@@ -442,6 +439,10 @@ bool ConsumeOperatorCopyableValuesChecker::check() {
442439
// TODO: We should add llvm.dbg.addr support for fastisel and also teach
443440
// CodeGen how to handle llvm.dbg.addr better.
444441
while (!valuesToProcess.empty()) {
442+
BitfieldRef<DiagnosticPrunedLiveness>::StackState livenessBitfieldContainer(
443+
livenessInfo.liveness, fn, nullptr,
444+
&livenessInfo.nonLifetimeEndingUsesInLiveOut);
445+
445446
auto lexicalValue = valuesToProcess.front();
446447
valuesToProcess = valuesToProcess.drop_front(1);
447448
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *lexicalValue);
@@ -473,7 +474,7 @@ bool ConsumeOperatorCopyableValuesChecker::check() {
473474
mvi->setAllowsDiagnostics(false);
474475

475476
LLVM_DEBUG(llvm::dbgs() << "Move Value: " << *mvi);
476-
if (livenessInfo.liveness.isWithinBoundary(mvi)) {
477+
if (livenessInfo.liveness->isWithinBoundary(mvi)) {
477478
LLVM_DEBUG(llvm::dbgs() << " WithinBoundary: Yes!\n");
478479
emitDiagnosticForMove(lexicalValue, varName, mvi);
479480
} else {

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)