Skip to content

Commit c6e2910

Browse files
Merge pull request #78015 from nate-chandler/cherrypick/release/6.1/rdar139842132
6.1: [OSSACanonicalizeOwned] Record traversed defs and don't traverse copies of guaranteed values.
2 parents f1a10a0 + 81242bb commit c6e2910

File tree

6 files changed

+210
-42
lines changed

6 files changed

+210
-42
lines changed

include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@
9696
#ifndef SWIFT_SILOPTIMIZER_UTILS_CANONICALOSSALIFETIME_H
9797
#define SWIFT_SILOPTIMIZER_UTILS_CANONICALOSSALIFETIME_H
9898

99-
#include "swift/Basic/GraphNodeWorklist.h"
10099
#include "swift/Basic/SmallPtrSetVector.h"
100+
#include "swift/Basic/TaggedUnion.h"
101101
#include "swift/SIL/PrunedLiveness.h"
102102
#include "swift/SIL/SILInstruction.h"
103103
#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
@@ -279,8 +279,71 @@ class CanonicalizeOSSALifetime final {
279279
/// outside the pruned liveness at the time it is discovered.
280280
llvm::SmallPtrSet<DebugValueInst *, 8> debugValues;
281281

282-
/// Visited set for general def-use traversal that prevents revisiting values.
283-
GraphNodeWorklist<SILValue, 8> defUseWorklist;
282+
class Def {
283+
struct Root {
284+
SILValue value;
285+
};
286+
struct Copy {
287+
CopyValueInst *cvi;
288+
};
289+
struct BorrowedFrom {
290+
BorrowedFromInst *bfi;
291+
};
292+
struct Reborrow {
293+
SILArgument *argument;
294+
};
295+
using Payload = TaggedUnion<Root, Copy, BorrowedFrom, Reborrow>;
296+
Payload payload;
297+
Def(Payload payload) : payload(payload) {}
298+
299+
public:
300+
enum class Kind {
301+
Root,
302+
Copy,
303+
BorrowedFrom,
304+
Reborrow,
305+
};
306+
Kind getKind() const {
307+
if (payload.isa<Root>()) {
308+
return Kind::Root;
309+
}
310+
if (payload.isa<Copy>()) {
311+
return Kind::Copy;
312+
}
313+
if (payload.isa<BorrowedFrom>()) {
314+
return Kind::BorrowedFrom;
315+
}
316+
assert(payload.isa<Reborrow>());
317+
return Kind::Reborrow;
318+
}
319+
operator Kind() const { return getKind(); }
320+
bool operator==(Def rhs) const {
321+
return getKind() == rhs.getKind() && getValue() == rhs.getValue();
322+
}
323+
static Def root(SILValue value) { return {Root{value}}; }
324+
static Def copy(CopyValueInst *cvi) { return {Copy{cvi}}; }
325+
static Def borrowedFrom(BorrowedFromInst *bfi) {
326+
return {BorrowedFrom{bfi}};
327+
}
328+
static Def reborrow(SILArgument *argument) { return {Reborrow{argument}}; }
329+
SILValue getValue() const {
330+
switch (*this) {
331+
case Kind::Root:
332+
return payload.get<Root>().value;
333+
case Kind::Copy:
334+
return payload.get<Copy>().cvi;
335+
case Kind::BorrowedFrom:
336+
return payload.get<BorrowedFrom>().bfi;
337+
case Kind::Reborrow:
338+
return payload.get<Reborrow>().argument;
339+
}
340+
llvm_unreachable("covered switch");
341+
}
342+
};
343+
friend llvm::DenseMapInfo<Def>;
344+
345+
/// The defs derived from currentDef whose uses are added to liveness.
346+
SmallVector<Def, 8> discoveredDefs;
284347

285348
/// The blocks that were discovered by PrunedLiveness.
286349
SmallVector<SILBasicBlock *, 32> discoveredBlocks;

lib/SIL/Utils/OSSALifetimeCompletion.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ bool OSSALifetimeCompletion::analyzeAndUpdateLifetime(SILValue value,
454454
namespace swift::test {
455455
// Arguments:
456456
// - SILValue: value
457+
// - string: either "liveness" or "availability"
457458
// Dumps:
458459
// - function
459460
static FunctionTest OSSALifetimeCompletionTest(

lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "swift/AST/DiagnosticsSIL.h"
1616
#include "swift/Basic/Assertions.h"
1717
#include "swift/Basic/Defer.h"
18+
#include "swift/Basic/GraphNodeWorklist.h"
1819
#include "swift/SIL/BasicBlockBits.h"
1920
#include "swift/SIL/BasicBlockDatastructures.h"
2021
#include "swift/SIL/DebugUtils.h"

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

Lines changed: 73 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@
6565

6666
#define DEBUG_TYPE "copy-propagation"
6767

68-
#include "swift/Basic/Assertions.h"
6968
#include "swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h"
69+
#include "swift/Basic/Assertions.h"
7070
#include "swift/SIL/InstructionUtils.h"
7171
#include "swift/SIL/NodeDatastructures.h"
7272
#include "swift/SIL/OSSALifetimeCompletion.h"
@@ -132,17 +132,29 @@ static bool isDestroyOfCopyOf(SILInstruction *instruction, SILValue def) {
132132
bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
133133
LLVM_DEBUG(llvm::dbgs() << "Computing canonical liveness from:\n";
134134
getCurrentDef()->print(llvm::dbgs()));
135-
defUseWorklist.initialize(getCurrentDef());
135+
SmallVector<unsigned, 8> indexWorklist;
136+
ValueSet visitedDefs(getCurrentDef()->getFunction());
137+
auto addDefToWorklist = [&](Def def) {
138+
if (!visitedDefs.insert(def.getValue()))
139+
return;
140+
discoveredDefs.push_back(def);
141+
indexWorklist.push_back(discoveredDefs.size() - 1);
142+
};
143+
discoveredDefs.clear();
144+
addDefToWorklist(Def::root(getCurrentDef()));
136145
// Only the first level of reborrows need to be consider. All nested inner
137146
// adjacent reborrows and phis are encapsulated within their lifetimes.
138147
SILPhiArgument *arg;
139148
if ((arg = dyn_cast<SILPhiArgument>(getCurrentDef())) && arg->isPhi()) {
140149
visitInnerAdjacentPhis(arg, [&](SILArgument *reborrow) {
141-
defUseWorklist.insert(reborrow);
150+
addDefToWorklist(Def::reborrow(reborrow));
142151
return true;
143152
});
144153
}
145-
while (SILValue value = defUseWorklist.pop()) {
154+
while (!indexWorklist.empty()) {
155+
auto index = indexWorklist.pop_back_val();
156+
auto def = discoveredDefs[index];
157+
auto value = def.getValue();
146158
LLVM_DEBUG(llvm::dbgs() << " Uses of value:\n";
147159
value->print(llvm::dbgs()));
148160

@@ -153,11 +165,20 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
153165
auto *user = use->getUser();
154166
// Recurse through copies.
155167
if (auto *copy = dyn_cast<CopyValueInst>(user)) {
156-
defUseWorklist.insert(copy);
168+
// Don't recurse through copies of borrowed-froms or reborrows.
169+
switch (def) {
170+
case Def::Kind::Root:
171+
case Def::Kind::Copy:
172+
addDefToWorklist(Def::copy(copy));
173+
break;
174+
case Def::Kind::Reborrow:
175+
case Def::Kind::BorrowedFrom:
176+
break;
177+
}
157178
continue;
158179
}
159180
if (auto *bfi = dyn_cast<BorrowedFromInst>(user)) {
160-
defUseWorklist.insert(bfi);
181+
addDefToWorklist(Def::borrowedFrom(bfi));
161182
continue;
162183
}
163184
// Handle debug_value instructions separately.
@@ -244,7 +265,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
244265
// This branch reborrows a guaranteed phi whose lifetime is dependent on
245266
// currentDef. Uses of the reborrowing phi extend liveness.
246267
auto *reborrow = PhiOperand(use).getValue();
247-
defUseWorklist.insert(reborrow);
268+
addDefToWorklist(Def::reborrow(reborrow));
248269
break;
249270
}
250271
}
@@ -1148,20 +1169,17 @@ void CanonicalizeOSSALifetime::rewriteCopies(
11481169
SmallVectorImpl<DestroyValueInst *> const &newDestroys) {
11491170
assert(getCurrentDef()->getOwnershipKind() == OwnershipKind::Owned);
11501171

1172+
// Shadow discoveredDefs in order to constrain its uses.
1173+
const auto &discoveredDefs = this->discoveredDefs;
1174+
11511175
InstructionSetVector instsToDelete(getCurrentDef()->getFunction());
1152-
defUseWorklist.clear();
11531176

11541177
// Visit each operand in the def-use chain.
11551178
//
11561179
// Return true if the operand can use the current definition. Return false if
11571180
// it requires a copy.
11581181
auto visitUse = [&](Operand *use) {
11591182
auto *user = use->getUser();
1160-
// Recurse through copies.
1161-
if (auto *copy = dyn_cast<CopyValueInst>(user)) {
1162-
defUseWorklist.insert(copy);
1163-
return true;
1164-
}
11651183
if (destroys.contains(user)) {
11661184
auto *destroy = cast<DestroyValueInst>(user);
11671185
// If this destroy was marked as a final destroy, ignore it; otherwise,
@@ -1195,37 +1213,54 @@ void CanonicalizeOSSALifetime::rewriteCopies(
11951213
};
11961214

11971215
// Perform a def-use traversal, visiting each use operand.
1198-
for (auto useIter = getCurrentDef()->use_begin(),
1199-
endIter = getCurrentDef()->use_end(); useIter != endIter;) {
1200-
Operand *use = *useIter++;
1201-
if (!visitUse(use)) {
1202-
copyLiveUse(use, getCallbacks());
1203-
}
1204-
}
1205-
while (SILValue value = defUseWorklist.pop()) {
1206-
CopyValueInst *srcCopy = cast<CopyValueInst>(value);
1207-
// Recurse through copies while replacing their uses.
1208-
Operand *reusedCopyOp = nullptr;
1209-
for (auto useIter = srcCopy->use_begin(); useIter != srcCopy->use_end();) {
1210-
Operand *use = *useIter++;
1211-
if (!visitUse(use)) {
1212-
if (!reusedCopyOp && srcCopy->getParent() == use->getParentBlock()) {
1213-
reusedCopyOp = use;
1214-
} else {
1216+
for (auto def : discoveredDefs) {
1217+
switch (def) {
1218+
case Def::Kind::BorrowedFrom:
1219+
case Def::Kind::Reborrow:
1220+
// Direct uses of these defs never need to be rewritten. Being guaranteed
1221+
// values, none of their direct uses consume an owned value.
1222+
assert(def.getValue()->getOwnershipKind() == OwnershipKind::Guaranteed);
1223+
break;
1224+
case Def::Kind::Root: {
1225+
SILValue value = def.getValue();
1226+
for (auto useIter = value->use_begin(), endIter = value->use_end();
1227+
useIter != endIter;) {
1228+
Operand *use = *useIter++;
1229+
if (!visitUse(use)) {
12151230
copyLiveUse(use, getCallbacks());
12161231
}
12171232
}
1233+
break;
12181234
}
1219-
if (!(reusedCopyOp && srcCopy->hasOneUse())) {
1220-
getCallbacks().replaceValueUsesWith(srcCopy, srcCopy->getOperand());
1221-
if (reusedCopyOp) {
1222-
reusedCopyOp->set(srcCopy);
1223-
} else {
1224-
if (instsToDelete.insert(srcCopy)) {
1225-
LLVM_DEBUG(llvm::dbgs() << " Removing " << *srcCopy);
1226-
++NumCopiesAndMovesEliminated;
1235+
case Def::Kind::Copy: {
1236+
SILValue value = def.getValue();
1237+
CopyValueInst *srcCopy = cast<CopyValueInst>(value);
1238+
// Recurse through copies while replacing their uses.
1239+
Operand *reusedCopyOp = nullptr;
1240+
for (auto useIter = srcCopy->use_begin();
1241+
useIter != srcCopy->use_end();) {
1242+
Operand *use = *useIter++;
1243+
if (!visitUse(use)) {
1244+
if (!reusedCopyOp && srcCopy->getParent() == use->getParentBlock()) {
1245+
reusedCopyOp = use;
1246+
} else {
1247+
copyLiveUse(use, getCallbacks());
1248+
}
12271249
}
12281250
}
1251+
if (!(reusedCopyOp && srcCopy->hasOneUse())) {
1252+
getCallbacks().replaceValueUsesWith(srcCopy, srcCopy->getOperand());
1253+
if (reusedCopyOp) {
1254+
reusedCopyOp->set(srcCopy);
1255+
} else {
1256+
if (instsToDelete.insert(srcCopy)) {
1257+
LLVM_DEBUG(llvm::dbgs() << " Removing " << *srcCopy);
1258+
++NumCopiesAndMovesEliminated;
1259+
}
1260+
}
1261+
}
1262+
break;
1263+
}
12291264
}
12301265
}
12311266
assert(!consumes.hasUnclaimedConsumes());

test/SILOptimizer/canonicalize_ossa_lifetime_unit.sil

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,3 +789,71 @@ entry(%c : @owned $C):
789789
specify_test "canonicalize_ossa_lifetime true false true %c"
790790
unreachable
791791
}
792+
793+
// CHECK-LABEL: begin running test {{.*}} on consume_copy_of_borrowed_from
794+
// CHECK-LABEL: sil [ossa] @consume_copy_of_borrowed_from : {{.*}} {
795+
// The copy can't be eliminated because it's a copy of a borrowed-from. Such
796+
// copies are not rewritten.
797+
// CHECK: copy_value
798+
// CHECK-LABEL: } // end sil function 'consume_copy_of_borrowed_from'
799+
// CHECK-LABEL: end running test {{.*}} on consume_copy_of_borrowed_from
800+
sil [ossa] @consume_copy_of_borrowed_from : $@convention(thin) (@owned C) -> () {
801+
bb0(%instance : @owned $C):
802+
%borrow = begin_borrow %instance : $C
803+
br bb1(%instance : $C, %borrow : $C)
804+
805+
bb1(%instance2 : @owned $C, %reborrowed : @reborrow $C):
806+
specify_test "canonicalize_ossa_lifetime true false true %instance2"
807+
%reborrow = borrowed %reborrowed : $C from (%instance2 : $C)
808+
cond_br undef, bb2, bb3
809+
810+
bb2:
811+
end_borrow %reborrow : $C
812+
destroy_value %instance2 : $C
813+
%retval = tuple ()
814+
return %retval : $()
815+
816+
bb3:
817+
%copy = copy_value %reborrow : $C
818+
%takeC = function_ref @takeC : $@convention(thin) (@owned C) -> ()
819+
apply %takeC(%copy) : $@convention(thin) (@owned C) -> ()
820+
unreachable
821+
}
822+
823+
// CHECK-LABEL: begin running test {{.*}} on consume_copy_of_borrowed_from_2
824+
// CHECK-LABEL: sil [ossa] @consume_copy_of_borrowed_from_2 : {{.*}} {
825+
// The copy can't be eliminated because it's a copy of a borrowed-from. Such
826+
// copies are not rewritten.
827+
// CHECK: copy_value
828+
// CHECK-LABEL: } // end sil function 'consume_copy_of_borrowed_from_2'
829+
// CHECK-LABEL: end running test {{.*}} on consume_copy_of_borrowed_from_2
830+
sil [ossa] @consume_copy_of_borrowed_from_2 : $@convention(thin) (@owned C) -> () {
831+
bb0(%instance : @owned $C):
832+
%borrow = begin_borrow %instance : $C
833+
br bb1(%instance : $C, %borrow : $C)
834+
835+
bb1(%instance2 : @owned $C, %reborrowed : @reborrow $C):
836+
specify_test "canonicalize_ossa_lifetime true false true %instance2"
837+
%reborrow = borrowed %reborrowed : $C from (%instance2 : $C)
838+
cond_br undef, bb2, bb3
839+
840+
bb2:
841+
end_borrow %reborrow : $C
842+
destroy_value %instance2 : $C
843+
br exit
844+
845+
bb3:
846+
%copy = copy_value %reborrow : $C
847+
end_borrow %reborrow : $C
848+
destroy_value %instance2 : $C
849+
br bb4
850+
851+
bb4:
852+
%takeC = function_ref @takeC : $@convention(thin) (@owned C) -> ()
853+
apply %takeC(%copy) : $@convention(thin) (@owned C) -> ()
854+
br exit
855+
856+
exit:
857+
%retval = tuple ()
858+
return %retval : $()
859+
}

test/SILOptimizer/copy_propagation_opaque.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ bb(%0 : @owned $T):
242242
// CHECK-ONONE-NEXT: [[CP:%.*]] = copy_value [[INSTANCE]] : $T
243243
// CHECK-NEXT: // function_ref takeMultipleOwned
244244
// CHECK-NEXT: function_ref @takeMultipleOwned : $@convention(thin) <τ_0_0> (@in τ_0_0, @in τ_0_0) -> ()
245-
// CHECK-NEXT: apply %{{.*}}<T>([[INSTANCE]], [[CP]]) : $@convention(thin) <τ_0_0> (@in τ_0_0, @in τ_0_0) -> ()
245+
// CHECK-NEXT: apply %{{.*}}<T>([[CP]], [[INSTANCE]]) : $@convention(thin) <τ_0_0> (@in τ_0_0, @in τ_0_0) -> ()
246246
// CHECK-ONONE-NEXT: destroy_value
247247
// CHECK-NEXT: tuple ()
248248
// CHECK-NEXT: return

0 commit comments

Comments
 (0)