Skip to content

Commit a4d31a4

Browse files
nate-chandlernate-chandler
committed
[ShrinkBorrowScope] Leave under rewritten copies.
When shrinking a borrow scope like %borrow = begin_borrow %value barrier %copy = copy_value %borrow end_borrow %borrow the copy will be rewritten to be a copy of the borrowee: %borrow = begin_borrow %value barrier %copy = copy_value %value ^^^^^^ end_borrow %borrow If, as here, shrinking next encounters a barrier, then the insertion point will be that rewritten copy_value instruction. In such a case, when rather than creating a new end_borrow there and deleting the old, just reuse the old one. The lifetime of the value being copied will be canonicalized by CopyPropagation regardless.
1 parent 4b85d0a commit a4d31a4

File tree

5 files changed

+138
-8
lines changed

5 files changed

+138
-8
lines changed

‎lib/SILOptimizer/UtilityPasses/UnitTestRunner.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
7676
#include "swift/SILOptimizer/PassManager/Passes.h"
7777
#include "swift/SILOptimizer/PassManager/Transforms.h"
78+
#include "swift/SILOptimizer/Utils/CanonicalizeBorrowScope.h"
7879
#include "swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h"
7980
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
8081
#include "swift/SILOptimizer/Utils/InstructionDeleter.h"
@@ -274,6 +275,36 @@ struct IsDeinitBarrierTest : UnitTest {
274275
}
275276
};
276277

278+
struct ShrinkBorrowScopeTest : UnitTest {
279+
ShrinkBorrowScopeTest(UnitTestRunner *pass) : UnitTest(pass) {}
280+
void invoke(Arguments &arguments) override {
281+
auto instruction = arguments.takeValue();
282+
auto expected = arguments.takeBool();
283+
auto *bbi = cast<BeginBorrowInst>(instruction);
284+
auto *analysis = getAnalysis<BasicCalleeAnalysis>();
285+
SmallVector<CopyValueInst *, 4> modifiedCopyValueInsts;
286+
InstructionDeleter deleter(
287+
InstModCallbacks().onDelete([&](auto *instruction) {
288+
llvm::errs() << "DELETED:\n";
289+
instruction->dump();
290+
}));
291+
auto shrunk =
292+
shrinkBorrowScope(*bbi, deleter, analysis, modifiedCopyValueInsts);
293+
unsigned index = 0;
294+
for (auto *cvi : modifiedCopyValueInsts) {
295+
auto expectedCopy = arguments.takeValue();
296+
llvm::errs() << "rewritten copy " << index << ":\n";
297+
llvm::errs() << "expected:\n";
298+
expectedCopy->print(llvm::errs());
299+
llvm::errs() << "got:\n";
300+
cvi->dump();
301+
assert(cvi == expectedCopy);
302+
++index;
303+
}
304+
assert(expected == shrunk && "didn't shrink expectedly!?");
305+
}
306+
};
307+
277308
/// [new_tests] Add the new UnitTest subclass above this line.
278309

279310
class UnitTestRunner : public SILFunctionTransform {
@@ -314,6 +345,7 @@ class UnitTestRunner : public SILFunctionTransform {
314345
ADD_UNIT_TEST_SUBCLASS(
315346
"pruned-liveness-boundary-with-list-of-last-users-insertion-points",
316347
PrunedLivenessBoundaryWithListOfLastUsersInsertionPointsTest)
348+
ADD_UNIT_TEST_SUBCLASS("shrink-borrow-scope", ShrinkBorrowScopeTest)
317349
ADD_UNIT_TEST_SUBCLASS("is-deinit-barrier", IsDeinitBarrierTest)
318350
/// [new_tests] Add the new mapping from string to subclass above this line.
319351

‎lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ class Rewriter final {
347347
bool run();
348348

349349
private:
350+
EndBorrowInst *findPreexistingEndBorrow(SILInstruction *instruction);
350351
bool createEndBorrow(SILInstruction *insertionPoint);
351352
};
352353

@@ -422,12 +423,31 @@ bool Rewriter::run() {
422423
return madeChange;
423424
}
424425

425-
bool Rewriter::createEndBorrow(SILInstruction *insertionPoint) {
426-
if (auto *ebi = dyn_cast<EndBorrowInst>(insertionPoint)) {
427-
if (llvm::find(uses.ends, insertionPoint) != uses.ends.end()) {
428-
reusedEndBorrowInsts.insert(insertionPoint);
429-
return false;
426+
EndBorrowInst *
427+
Rewriter::findPreexistingEndBorrow(SILInstruction *insertionPoint) {
428+
for (auto *instruction = insertionPoint; instruction;
429+
instruction = instruction->getNextInstruction()) {
430+
if (auto *ebi = dyn_cast<EndBorrowInst>(instruction)) {
431+
if (llvm::find(uses.ends, ebi) != uses.ends.end())
432+
return ebi;
433+
}
434+
if (auto *cvi = dyn_cast<CopyValueInst>(instruction)) {
435+
if (llvm::is_contained(barriers.copies, cvi)) {
436+
continue;
437+
}
430438
}
439+
/// Otherwise, this is an "interesting" instruction. We want to record that
440+
/// we were able to hoist the end of the borrow scope over it, so we stop
441+
/// looking for the preexisting end_borrow.
442+
return nullptr;
443+
}
444+
return nullptr;
445+
}
446+
447+
bool Rewriter::createEndBorrow(SILInstruction *insertionPoint) {
448+
if (auto *ebi = findPreexistingEndBorrow(insertionPoint)) {
449+
reusedEndBorrowInsts.insert(ebi);
450+
return false;
431451
}
432452
auto builder = SILBuilderWithScope(insertionPoint);
433453
builder.createEndBorrow(

‎test/SILOptimizer/copy_propagation_opaque.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,8 +413,8 @@ bb0:
413413

414414
// CHECK-TRACE-LABEL: *** CopyPropagation: testBorrowCopy
415415
// CHECK-TRACE: Canonicalizing: %1
416-
// CHECK-TRACE: Removing destroy_value %4 : $T
417-
// CHECK-TRACE: Removing %4 = copy_value %1 : $T
416+
// CHECK-TRACE: Removing destroy_value %3 : $T
417+
// CHECK-TRACE: Removing %3 = copy_value %1 : $T
418418
//
419419
// CHECK-LABEL: sil [ossa] @testBorrowCopy : {{.*}} {
420420
// CHECK-LABEL: bb0:

‎test/SILOptimizer/shrink_borrow_scope.sil

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -804,12 +804,12 @@ entry(%instance : @owned $C):
804804
%_ = apply %callee_guaranteed(%lifetime_1) : $@convention(thin) (@guaranteed C) -> ()
805805
%synchronized = apply %synchronization_point() : $@convention(thin) () -> ()
806806
%copy_2 = copy_value %copy_1 : $C
807+
%result = tuple ()
807808
end_borrow %lifetime_1 : $C
808809
%0 = apply %callee_owned(%instance) : $@convention(thin) (@owned C) -> ()
809810
%1 = apply %callee_owned(%copy_1) : $@convention(thin) (@owned C) -> ()
810811
%2 = apply %callee_owned(%copy_2) : $@convention(thin) (@owned C) -> ()
811812

812-
%result = tuple ()
813813
return %result : $()
814814
}
815815

@@ -1122,6 +1122,54 @@ entry:
11221122
return %retval : $()
11231123
}
11241124

1125+
// Don't hoist over a copy_value that is rewritten.
1126+
//
1127+
// The borrowee's lifetime will be canonicalized again regardless.
1128+
//
1129+
// CHECK-LABEL: sil [ossa] @nohoist_over_rewritten_copy : {{.*}} {
1130+
// CHECK: [[INSTANCE:%[^,]+]] = apply
1131+
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [lexical] [[INSTANCE]]
1132+
// CHECK: [[COPY:%[^,]+]] = copy_value [[INSTANCE]]
1133+
// CHECK: end_borrow [[LIFETIME]]
1134+
// CHECK: tuple ([[COPY]] : $C, [[INSTANCE]] : $C)
1135+
// CHECK-LABEL: } // end sil function 'nohoist_over_rewritten_copy'
1136+
sil [ossa] @nohoist_over_rewritten_copy : $@convention(thin) () -> (@owned C, @owned C) {
1137+
%get_owned_c = function_ref @get_owned_c : $@convention(thin) () -> (@owned C)
1138+
%instance = apply %get_owned_c() : $@convention(thin) () -> (@owned C)
1139+
%lifetime = begin_borrow [lexical] %instance : $C
1140+
%callee_guaranteed = function_ref @callee_guaranteed : $@convention(thin) (@guaranteed C) -> ()
1141+
apply %callee_guaranteed(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
1142+
%copy = copy_value %lifetime : $C
1143+
end_borrow %lifetime : $C
1144+
%retval = tuple (%copy : $C, %instance : $C)
1145+
return %retval : $(C, C)
1146+
}
1147+
1148+
// Don't hoist over multiple copy_values that are rewritten.
1149+
//
1150+
// The borrowee's lifetime will be canonicalized again regardless.
1151+
//
1152+
// CHECK-LABEL: sil [ossa] @nohoist_over_rewritten_copy_multiple : {{.*}} {
1153+
// CHECK: [[INSTANCE:%[^,]+]] = apply
1154+
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [lexical] [[INSTANCE]]
1155+
// CHECK: [[COPY1:%[^,]+]] = copy_value [[INSTANCE]]
1156+
// CHECK: [[COPY2:%[^,]+]] = copy_value [[INSTANCE]]
1157+
// CHECK: end_borrow [[LIFETIME]]
1158+
// CHECK: tuple ([[COPY2]] : $C, [[COPY1]] : $C, [[INSTANCE]] : $C)
1159+
// CHECK-LABEL: } // end sil function 'nohoist_over_rewritten_copy_multiple'
1160+
sil [ossa] @nohoist_over_rewritten_copy_multiple : $@convention(thin) () -> (@owned C, @owned C, @owned C) {
1161+
%get_owned_c = function_ref @get_owned_c : $@convention(thin) () -> (@owned C)
1162+
%instance = apply %get_owned_c() : $@convention(thin) () -> (@owned C)
1163+
%lifetime = begin_borrow [lexical] %instance : $C
1164+
%callee_guaranteed = function_ref @callee_guaranteed : $@convention(thin) (@guaranteed C) -> ()
1165+
apply %callee_guaranteed(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
1166+
%copy1 = copy_value %lifetime : $C
1167+
%copy2 = copy_value %lifetime : $C
1168+
end_borrow %lifetime : $C
1169+
%retval = tuple (%copy2 : $C, %copy1 : $C, %instance : $C)
1170+
return %retval : $(C, C, C)
1171+
}
1172+
11251173
// =============================================================================
11261174
// instruction tests }}
11271175
// =============================================================================

‎test/SILOptimizer/shrink_borrow_scope.unit.sil

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %target-sil-opt -unit-test-runner %s 2>&1 | %FileCheck %s
2+
3+
import Builtin
4+
import Swift
5+
6+
class C {}
7+
8+
sil [ossa] @get_owned_c : $@convention(thin) () -> (@owned C)
9+
sil [ossa] @callee_guaranteed: $@convention(thin) (@guaranteed C) -> ()
10+
11+
// CHECK-LABEL: begin running test 1 of {{[^,]+}} on nohoist_over_rewritten_copy
12+
// CHECK-NOT: DELETED:
13+
// CHECK-NOT: end_borrow
14+
// CHECK-LABEL: end running test 1 of {{[^,]+}} on nohoist_over_rewritten_copy
15+
sil [ossa] @nohoist_over_rewritten_copy : $@convention(thin) () -> (@owned C, @owned C) {
16+
entry:
17+
test_specification "shrink-borrow-scope @trace true @trace[1]"
18+
%get_owned_c = function_ref @get_owned_c : $@convention(thin) () -> (@owned C)
19+
%instance = apply %get_owned_c() : $@convention(thin) () -> (@owned C)
20+
%lifetime = begin_borrow [lexical] %instance : $C
21+
debug_value [trace] %lifetime : $C
22+
%callee_guaranteed = function_ref @callee_guaranteed : $@convention(thin) (@guaranteed C) -> ()
23+
apply %callee_guaranteed(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
24+
%copy = copy_value %lifetime : $C
25+
debug_value [trace] %copy : $C
26+
end_borrow %lifetime : $C
27+
%retval = tuple (%copy : $C, %instance : $C)
28+
return %retval : $(C, C)
29+
}
30+

0 commit comments

Comments
 (0)