Skip to content

Commit 21069bb

Browse files
committed
[ShrinkBorrowScope] Avoid spurious mutation.
If an end_borrow cannot be hoisted, do not rewrite it.
1 parent 12e2663 commit 21069bb

File tree

1 file changed

+50
-13
lines changed

1 file changed

+50
-13
lines changed

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,34 @@ class ShrinkBorrowScope {
7676
// The instructions from which the shrinking starts, the scope ending
7777
// instructions, keyed off the block in which they appear.
7878
llvm::SmallDenseMap<SILBasicBlock *, SILInstruction *> startingInstructions;
79+
// The end _borrow instructions for this borrow scope that existed before
80+
// ShrinkBorrowScope ran and which were not modified.
81+
llvm::SmallPtrSet<SILInstruction *, 8> reusedEndBorrowInsts;
82+
83+
// Whether ShrinkBorrowScope made any changes to the function.
84+
//
85+
// It could have made one of the following sorts of changes:
86+
// - deleted an end_borrow
87+
// - created an end_borrow
88+
// - rewrote the operand of an instruction
89+
// - ApplySite
90+
// - begin_borrow
91+
// - copy_value
92+
bool madeChange;
7993

8094
public:
8195
ShrinkBorrowScope(BeginBorrowInst *bbi, InstructionDeleter &deleter,
8296
SmallVectorImpl<CopyValueInst *> &modifiedCopyValueInsts)
8397
: introducer(bbi), deleter(deleter),
84-
modifiedCopyValueInsts(modifiedCopyValueInsts) {}
98+
modifiedCopyValueInsts(modifiedCopyValueInsts), madeChange(false) {}
8599

86100
bool run();
87101

88102
bool populateUsers();
89103
bool initializeWorklist();
90104
void findBarriers();
91-
void rewrite();
92-
void createEndBorrow(SILInstruction *insertionPoint);
105+
bool rewrite();
106+
bool createEndBorrow(SILInstruction *insertionPoint);
93107

94108
bool isBarrierApply(SILInstruction *instruction) {
95109
// For now, treat every apply (that doesn't use the borrowed value) as a
@@ -165,8 +179,10 @@ class ShrinkBorrowScope {
165179
if (auto *cvi = dyn_cast<CopyValueInst>(argument)) {
166180
cvi->setOperand(borrowee);
167181
modifiedCopyValueInsts.push_back(cvi);
182+
madeChange = true;
168183
} else {
169184
apply.setArgument(index, borrowee);
185+
madeChange = true;
170186
}
171187
}
172188
return true;
@@ -175,12 +191,14 @@ class ShrinkBorrowScope {
175191
canReplaceValueWithBorrowedValue(bbi->getOperand())) {
176192
auto borrowee = introducer->getOperand();
177193
bbi->setOperand(borrowee);
194+
madeChange = true;
178195
return true;
179196
}
180197
} else if (auto *cvi = dyn_cast<CopyValueInst>(instruction)) {
181198
if (canReplaceValueWithBorrowedValue(cvi->getOperand())) {
182199
auto borrowee = introducer->getOperand();
183200
cvi->setOperand(borrowee);
201+
madeChange = true;
184202
modifiedCopyValueInsts.push_back(cvi);
185203
return true;
186204
}
@@ -205,9 +223,9 @@ bool ShrinkBorrowScope::run() {
205223

206224
findBarriers();
207225

208-
rewrite();
226+
madeChange |= rewrite();
209227

210-
return true;
228+
return madeChange;
211229
}
212230

213231
bool ShrinkBorrowScope::populateUsers() {
@@ -311,31 +329,50 @@ void ShrinkBorrowScope::findBarriers() {
311329
}
312330
}
313331

314-
void ShrinkBorrowScope::rewrite() {
315-
// Remove all the original end_borrow instructions.
316-
for (auto pair : startingInstructions) {
317-
deleter.forceDelete(pair.getSecond());
318-
}
332+
bool ShrinkBorrowScope::rewrite() {
333+
bool createdBorrow = false;
319334

320335
// Insert the new end_borrow instructions that occur after deinit barriers.
321336
for (auto pair : barrierInstructions) {
322337
auto *insertionPoint = getNextInstruction(pair.second);
323-
createEndBorrow(insertionPoint);
338+
createdBorrow |= createEndBorrow(insertionPoint);
324339
}
325340

326341
// Insert the new end_borrow instructions that occur at the beginning of
327342
// blocks which we couldn't hoist out of.
328343
for (auto *block : barredBlocks) {
329344
auto *insertionPoint = &*block->begin();
330-
createEndBorrow(insertionPoint);
345+
createdBorrow |= createEndBorrow(insertionPoint);
346+
}
347+
348+
if (createdBorrow) {
349+
// Remove all the original end_borrow instructions.
350+
for (auto pair : startingInstructions) {
351+
if (reusedEndBorrowInsts.contains(pair.second)) {
352+
continue;
353+
}
354+
deleter.forceDelete(pair.getSecond());
355+
}
331356
}
357+
358+
return createdBorrow;
332359
}
333360

334-
void ShrinkBorrowScope::createEndBorrow(SILInstruction *insertionPoint) {
361+
bool ShrinkBorrowScope::createEndBorrow(SILInstruction *insertionPoint) {
362+
if (auto *ebi = dyn_cast<EndBorrowInst>(insertionPoint)) {
363+
llvm::SmallDenseMap<SILBasicBlock *, SILInstruction *>::iterator location;
364+
if ((location = llvm::find_if(startingInstructions, [&](auto pair) -> bool {
365+
return pair.second == insertionPoint;
366+
})) != startingInstructions.end()) {
367+
reusedEndBorrowInsts.insert(location->second);
368+
return false;
369+
}
370+
}
335371
auto builder = SILBuilderWithScope(insertionPoint);
336372
builder.createEndBorrow(
337373
RegularLocation::getAutoGeneratedLocation(insertionPoint->getLoc()),
338374
introducer);
375+
return true;
339376
}
340377

341378
bool swift::shrinkBorrowScope(

0 commit comments

Comments
 (0)