Skip to content

Commit cd54bbe

Browse files
committed
[OSSACanonicalizeOwned] Only discover defs once.
The utility performs two def-use traversals. The first determines liveness from uses. The second rewrites copies. Previously, the defs whose uses were analyzed were discovered twice, once during each traversal. The non-triviality of the discovery logic (i.e. the logic determining when to walk into the values produced by the insructions which were the users of visited uses) opened the possibility for a divergence between the two discoveries. This possibility had indeed been realized--the two discoveries didn't visit exactly the same uses, and issues ensue. Here, the defs whose uses are analyzed are discovered only once (and not discarded as their uses are analyzed) during the first traversal. The second traversal reuses the defs discovered in the first traversal, eliminating the possibility of a def discovery difference. Furthermore, the defs are traversed in the same order.
1 parent bfcb701 commit cd54bbe

File tree

2 files changed

+8
-15
lines changed

2 files changed

+8
-15
lines changed

include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,9 @@ class CanonicalizeOSSALifetime final {
345345
/// The defs derived from currentDef whose uses are added to liveness.
346346
llvm::SmallSetVector<Def, 8> discoveredDefs;
347347

348+
/// The indices in discoveredDefs in the order in which they are visited.
349+
SmallVector<unsigned, 8> discoveredDefIndices;
350+
348351
/// The blocks that were discovered by PrunedLiveness.
349352
SmallVector<SILBasicBlock *, 32> discoveredBlocks;
350353

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
139139
indexWorklist.push_back(discoveredDefs.size() - 1);
140140
};
141141
discoveredDefs.clear();
142+
discoveredDefIndices.clear();
142143
addDefToWorklist(Def::root(getCurrentDef()));
143144
// Only the first level of reborrows need to be consider. All nested inner
144145
// adjacent reborrows and phis are encapsulated within their lifetimes.
@@ -151,6 +152,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
151152
}
152153
while (!indexWorklist.empty()) {
153154
auto index = indexWorklist.pop_back_val();
155+
discoveredDefIndices.push_back(index);
154156
auto def = discoveredDefs[index];
155157
auto value = def.getValue();
156158
LLVM_DEBUG(llvm::dbgs() << " Uses of value:\n";
@@ -1159,14 +1161,7 @@ void CanonicalizeOSSALifetime::rewriteCopies(
11591161
assert(getCurrentDef()->getOwnershipKind() == OwnershipKind::Owned);
11601162

11611163
// Shadow discoveredDefs in order to constrain its uses.
1162-
auto &discoveredDefs = this->discoveredDefs;
1163-
1164-
SmallVector<unsigned, 8> indexWorklist;
1165-
auto addDefToWorklist = [&](Def def) {
1166-
if (!discoveredDefs.insert(def))
1167-
return;
1168-
indexWorklist.push_back(discoveredDefs.size() - 1);
1169-
};
1164+
const auto &discoveredDefs = this->discoveredDefs;
11701165

11711166
InstructionSetVector instsToDelete(getCurrentDef()->getFunction());
11721167

@@ -1176,9 +1171,8 @@ void CanonicalizeOSSALifetime::rewriteCopies(
11761171
// it requires a copy.
11771172
auto visitUse = [&](Operand *use) {
11781173
auto *user = use->getUser();
1179-
// Recurse through copies.
1174+
// The current def can always be used for a copy_value.
11801175
if (auto *copy = dyn_cast<CopyValueInst>(user)) {
1181-
addDefToWorklist(Def::copy(copy));
11821176
return true;
11831177
}
11841178
if (destroys.contains(user)) {
@@ -1213,12 +1207,8 @@ void CanonicalizeOSSALifetime::rewriteCopies(
12131207
return true;
12141208
};
12151209

1216-
discoveredDefs.clear();
1217-
addDefToWorklist(Def::root(getCurrentDef()));
12181210
// Perform a def-use traversal, visiting each use operand.
1219-
1220-
while (!indexWorklist.empty()) {
1221-
auto index = indexWorklist.pop_back_val();
1211+
for (auto index : discoveredDefIndices) {
12221212
auto def = discoveredDefs[index];
12231213
switch (def) {
12241214
case Def::Kind::BorrowedFrom:

0 commit comments

Comments
 (0)