Skip to content

Commit 39c5a2e

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 instructions 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 traversals 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 eebe9ac commit 39c5a2e

File tree

2 files changed

+7
-20
lines changed

2 files changed

+7
-20
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
SmallVector<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: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
141141
indexWorklist.push_back(discoveredDefs.size() - 1);
142142
};
143143
discoveredDefs.clear();
144+
discoveredDefIndices.clear();
144145
addDefToWorklist(Def::root(getCurrentDef()));
145146
// Only the first level of reborrows need to be consider. All nested inner
146147
// adjacent reborrows and phis are encapsulated within their lifetimes.
@@ -153,6 +154,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
153154
}
154155
while (!indexWorklist.empty()) {
155156
auto index = indexWorklist.pop_back_val();
157+
discoveredDefIndices.push_back(index);
156158
auto def = discoveredDefs[index];
157159
auto value = def.getValue();
158160
LLVM_DEBUG(llvm::dbgs() << " Uses of value:\n";
@@ -1161,16 +1163,7 @@ void CanonicalizeOSSALifetime::rewriteCopies(
11611163
assert(getCurrentDef()->getOwnershipKind() == OwnershipKind::Owned);
11621164

11631165
// Shadow discoveredDefs in order to constrain its uses.
1164-
auto &discoveredDefs = this->discoveredDefs;
1165-
1166-
SmallVector<unsigned, 8> indexWorklist;
1167-
ValueSet visitedDefs(getCurrentDef()->getFunction());
1168-
auto addDefToWorklist = [&](Def def) {
1169-
if (!visitedDefs.insert(def.getValue()))
1170-
return;
1171-
discoveredDefs.push_back(def);
1172-
indexWorklist.push_back(discoveredDefs.size() - 1);
1173-
};
1166+
const auto &discoveredDefs = this->discoveredDefs;
11741167

11751168
InstructionSetVector instsToDelete(getCurrentDef()->getFunction());
11761169

@@ -1180,11 +1173,6 @@ void CanonicalizeOSSALifetime::rewriteCopies(
11801173
// it requires a copy.
11811174
auto visitUse = [&](Operand *use) {
11821175
auto *user = use->getUser();
1183-
// Recurse through copies.
1184-
if (auto *copy = dyn_cast<CopyValueInst>(user)) {
1185-
addDefToWorklist(Def::copy(copy));
1186-
return true;
1187-
}
11881176
if (destroys.contains(user)) {
11891177
auto *destroy = cast<DestroyValueInst>(user);
11901178
// If this destroy was marked as a final destroy, ignore it; otherwise,
@@ -1217,12 +1205,8 @@ void CanonicalizeOSSALifetime::rewriteCopies(
12171205
return true;
12181206
};
12191207

1220-
discoveredDefs.clear();
1221-
addDefToWorklist(Def::root(getCurrentDef()));
12221208
// Perform a def-use traversal, visiting each use operand.
1223-
1224-
while (!indexWorklist.empty()) {
1225-
auto index = indexWorklist.pop_back_val();
1209+
for (auto index : discoveredDefIndices) {
12261210
auto def = discoveredDefs[index];
12271211
switch (def) {
12281212
case Def::Kind::BorrowedFrom:

0 commit comments

Comments
 (0)