Skip to content

Commit b4092c4

Browse files
committed
[move-only] Remove use after move in CopiedLoadBorrowEliminationVisitor.
Specifically, we were calling process on a visitor that was already moved. To fix this I created a separate state structure that the visitor initializes and moved the post-processing step onto the state data structure. This seems to not be causing any problems today, but could in the future. rdar://111060475
1 parent dd28827 commit b4092c4

File tree

1 file changed

+41
-33
lines changed

1 file changed

+41
-33
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,15 +1433,49 @@ class ExtendUnconsumedLiveness {
14331433

14341434
namespace {
14351435

1436+
struct CopiedLoadBorrowEliminationState {
1437+
SILFunction *fn;
1438+
StackList<LoadBorrowInst *> targets;
1439+
1440+
CopiedLoadBorrowEliminationState(SILFunction *fn) : fn(fn), targets(fn) {}
1441+
1442+
void process() {
1443+
if (targets.empty())
1444+
return;
1445+
1446+
while (!targets.empty()) {
1447+
auto *lbi = targets.pop_back_val();
1448+
SILBuilderWithScope builder(lbi);
1449+
SILValue li = builder.emitLoadValueOperation(
1450+
lbi->getLoc(), lbi->getOperand(), LoadOwnershipQualifier::Copy);
1451+
SILValue borrow = builder.createBeginBorrow(lbi->getLoc(), li);
1452+
1453+
for (auto *ebi : lbi->getEndBorrows()) {
1454+
auto *next = ebi->getNextInstruction();
1455+
SILBuilderWithScope builder(next);
1456+
auto loc = RegularLocation::getAutoGeneratedLocation();
1457+
builder.emitDestroyValueOperation(loc, li);
1458+
}
1459+
1460+
lbi->replaceAllUsesWith(borrow);
1461+
lbi->eraseFromParent();
1462+
}
1463+
1464+
LLVM_DEBUG(llvm::dbgs() << "After Load Borrow Elim. Func Dump Start! ";
1465+
fn->print(llvm::dbgs()));
1466+
LLVM_DEBUG(llvm::dbgs() << "After Load Borrow Elim. Func Dump End!\n");
1467+
}
1468+
};
1469+
14361470
/// An early transform that we run to convert any load_borrow that are copied
14371471
/// directly or that have any subelement that is copied to a load [copy]. This
14381472
/// lets the rest of the optimization handle these as appropriate.
14391473
struct CopiedLoadBorrowEliminationVisitor final
14401474
: public TransitiveAddressWalker {
1441-
SILFunction *fn;
1442-
StackList<LoadBorrowInst *> targets;
1475+
CopiedLoadBorrowEliminationState &state;
14431476

1444-
CopiedLoadBorrowEliminationVisitor(SILFunction *fn) : fn(fn), targets(fn) {}
1477+
CopiedLoadBorrowEliminationVisitor(CopiedLoadBorrowEliminationState &state)
1478+
: state(state) {}
14451479

14461480
bool visitUse(Operand *op) override {
14471481
LLVM_DEBUG(llvm::dbgs() << "CopiedLBElim visiting ";
@@ -1527,36 +1561,9 @@ struct CopiedLoadBorrowEliminationVisitor final
15271561
if (!shouldConvertToLoadCopy)
15281562
return true;
15291563

1530-
targets.push_back(lbi);
1564+
state.targets.push_back(lbi);
15311565
return true;
15321566
}
1533-
1534-
void process() {
1535-
if (targets.empty())
1536-
return;
1537-
1538-
while (!targets.empty()) {
1539-
auto *lbi = targets.pop_back_val();
1540-
SILBuilderWithScope builder(lbi);
1541-
SILValue li = builder.emitLoadValueOperation(
1542-
lbi->getLoc(), lbi->getOperand(), LoadOwnershipQualifier::Copy);
1543-
SILValue borrow = builder.createBeginBorrow(lbi->getLoc(), li);
1544-
1545-
for (auto *ebi : lbi->getEndBorrows()) {
1546-
auto *next = ebi->getNextInstruction();
1547-
SILBuilderWithScope builder(next);
1548-
auto loc = RegularLocation::getAutoGeneratedLocation();
1549-
builder.emitDestroyValueOperation(loc, li);
1550-
}
1551-
1552-
lbi->replaceAllUsesWith(borrow);
1553-
lbi->eraseFromParent();
1554-
}
1555-
1556-
LLVM_DEBUG(llvm::dbgs() << "After Load Borrow Elim. Func Dump Start! ";
1557-
fn->print(llvm::dbgs()));
1558-
LLVM_DEBUG(llvm::dbgs() << "After Load Borrow Elim. Func Dump End!\n");
1559-
}
15601567
};
15611568

15621569
} // namespace
@@ -3130,14 +3137,15 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
31303137
// [copy] + begin_borrow for further processing. This just eliminates a case
31313138
// that the checker doesn't need to know about.
31323139
{
3133-
CopiedLoadBorrowEliminationVisitor copiedLoadBorrowEliminator(fn);
3140+
CopiedLoadBorrowEliminationState state(markedAddress->getFunction());
3141+
CopiedLoadBorrowEliminationVisitor copiedLoadBorrowEliminator(state);
31343142
if (AddressUseKind::Unknown ==
31353143
std::move(copiedLoadBorrowEliminator).walk(markedAddress)) {
31363144
LLVM_DEBUG(llvm::dbgs() << "Failed copied load borrow eliminator visit: "
31373145
<< *markedAddress);
31383146
return false;
31393147
}
3140-
copiedLoadBorrowEliminator.process();
3148+
state.process();
31413149
}
31423150

31433151
// Then gather all uses of our address by walking from def->uses. We use this

0 commit comments

Comments
 (0)