Skip to content

Commit 44e89fb

Browse files
aschwaighoferaschwaighofer
committed
UnsafeGuaranteedPeephole should handle virtual_method calls
Make the peephole stronger to handle retain(self), apply(self), release(self) pairs inbetween unsafeGuaranteed pairs. rdar://30948468
1 parent 7e3e7c0 commit 44e89fb

File tree

4 files changed

+152
-14
lines changed

4 files changed

+152
-14
lines changed

‎include/swift/SILOptimizer/Analysis/ARCAnalysis.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -375,19 +375,18 @@ getSingleUnsafeGuaranteedValueResult(BuiltinInst *UnsafeGuaranteedInst);
375375
/// "unsafeGuaranteed"'s token.
376376
BuiltinInst *getUnsafeGuaranteedEndUser(SILInstruction *UnsafeGuaranteedToken);
377377

378-
/// Walk backwards from an unsafeGuaranteedEnd builtin instruction looking for a
378+
/// Walk forwards from an unsafeGuaranteedEnd builtin instruction looking for a
379379
/// release on the reference returned by the matching unsafeGuaranteed builtin
380380
/// ignoring releases on the way.
381381
/// Return nullptr if no release is found.
382382
///
383383
/// %4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
384384
/// %5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
385385
/// %6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
386-
/// strong_release %5 : $Foo // <-- Matching release.
387-
/// strong_release %6 : $Foo // Ignore.
388386
/// %12 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
387+
/// strong_release %5 : $Foo // <-- Matching release.
389388
///
390-
/// Alternatively, look for the release after the unsafeGuaranteedEnd.
389+
/// Alternatively, look for the release before the unsafeGuaranteedEnd.
391390
SILInstruction *findReleaseToMatchUnsafeGuaranteedValue(
392391
SILInstruction *UnsafeGuaranteedEndI, SILInstruction *UnsafeGuaranteedI,
393392
SILValue UnsafeGuaranteedValue, SILBasicBlock &BB,

‎lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,11 +1133,11 @@ SILInstruction *swift::findReleaseToMatchUnsafeGuaranteedValue(
11331133
auto UnsafeGuaranteedOpdRoot =
11341134
RCFI.getRCIdentityRoot(UnsafeGuaranteedI->getOperand(0));
11351135

1136-
// Look before the "unsafeGuaranteedEnd".
1137-
for (auto ReverseIt = ++UnsafeGuaranteedEndI->getIterator().getReverse(),
1138-
End = BB.rend();
1139-
ReverseIt != End; ++ReverseIt) {
1140-
SILInstruction &CurInst = *ReverseIt;
1136+
// Try finding it after the "unsafeGuaranteedEnd".
1137+
for (auto ForwardIt = std::next(UnsafeGuaranteedEndI->getIterator()),
1138+
End = BB.end();
1139+
ForwardIt != End; ++ForwardIt) {
1140+
SILInstruction &CurInst = *ForwardIt;
11411141

11421142
// Is this a release?
11431143
if (isa<ReleaseValueInst>(CurInst) || isa<StrongReleaseInst>(CurInst)) {
@@ -1152,11 +1152,11 @@ SILInstruction *swift::findReleaseToMatchUnsafeGuaranteedValue(
11521152
break;
11531153
}
11541154

1155-
// Otherwise, try finding it after the "unsafeGuaranteedEnd".
1156-
for (auto ForwardIt = std::next(UnsafeGuaranteedEndI->getIterator()),
1157-
End = BB.end();
1158-
ForwardIt != End; ++ForwardIt) {
1159-
SILInstruction &CurInst = *ForwardIt;
1155+
// Otherwise, Look before the "unsafeGuaranteedEnd".
1156+
for (auto ReverseIt = ++UnsafeGuaranteedEndI->getIterator().getReverse(),
1157+
End = BB.rend();
1158+
ReverseIt != End; ++ReverseIt) {
1159+
SILInstruction &CurInst = *ReverseIt;
11601160

11611161
// Is this a release?
11621162
if (isa<ReleaseValueInst>(CurInst) || isa<StrongReleaseInst>(CurInst)) {
@@ -1170,6 +1170,7 @@ SILInstruction *swift::findReleaseToMatchUnsafeGuaranteedValue(
11701170
!isa<DebugValueAddrInst>(CurInst))
11711171
break;
11721172
}
1173+
11731174
return nullptr;
11741175
}
11751176

‎lib/SILOptimizer/Transforms/UnsafeGuaranteedPeephole.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,56 @@
4444

4545
using namespace swift;
4646

47+
/// Pattern match and remove "retain(self), apply(self), release(self)" calls
48+
/// inbetween unsafeGuaranteed pairs and remove the retain/release pairs.
49+
static void tryRemoveRetainReleasePairsBetween(
50+
RCIdentityFunctionInfo &RCFI, SILInstruction *UnsafeGuaranteedI,
51+
SILInstruction *Retain, SILInstruction *Release,
52+
SILInstruction *UnsafeGuaranteedEndI) {
53+
auto *BB = UnsafeGuaranteedI->getParent();
54+
if (BB != UnsafeGuaranteedEndI->getParent() || BB != Retain->getParent() ||
55+
BB != Release->getParent())
56+
return;
57+
58+
SILInstruction *CandidateRetain = nullptr;
59+
SmallVector<SILInstruction *, 8> InstsToDelete;
60+
61+
SILBasicBlock::iterator It(UnsafeGuaranteedI);
62+
while (It != BB->end() && It != SILBasicBlock::iterator(Release) &&
63+
It != SILBasicBlock::iterator(UnsafeGuaranteedEndI)) {
64+
auto *CurInst = &*It++;
65+
if (CurInst != Retain &&
66+
(isa<StrongRetainInst>(CurInst) || isa<RetainValueInst>(CurInst)) &&
67+
RCFI.getRCIdentityRoot(CurInst->getOperand(0)) ==
68+
SILValue(UnsafeGuaranteedI)) {
69+
CandidateRetain = CurInst;
70+
continue;
71+
}
72+
if (!CurInst->mayHaveSideEffects())
73+
continue;
74+
75+
if (isa<DebugValueInst>(CurInst) || isa<DebugValueAddrInst>(CurInst))
76+
continue;
77+
78+
if (isa<ApplyInst>(CurInst) || isa<PartialApplyInst>(CurInst))
79+
continue;
80+
81+
if (CandidateRetain != nullptr && CurInst != Release &&
82+
(isa<StrongReleaseInst>(CurInst) || isa<ReleaseValueInst>(CurInst)) &&
83+
RCFI.getRCIdentityRoot(CurInst->getOperand(0)) ==
84+
SILValue(UnsafeGuaranteedI)) {
85+
// Delete the retain/release pair.
86+
InstsToDelete.push_back(CandidateRetain);
87+
InstsToDelete.push_back(CurInst);
88+
}
89+
90+
// Otherwise, reset our scan.
91+
CandidateRetain = nullptr;
92+
}
93+
for (auto *Inst: InstsToDelete)
94+
Inst->eraseFromParent();
95+
}
96+
4797
/// Remove retain/release pairs around builtin "unsafeGuaranteed" instruction
4898
/// sequences.
4999
static bool removeGuaranteedRetainReleasePairs(SILFunction &F,
@@ -154,6 +204,12 @@ static bool removeGuaranteedRetainReleasePairs(SILFunction &F,
154204
// Okay we found a post dominating release. Let's remove the
155205
// retain/unsafeGuaranteed/release combo.
156206
//
207+
// Before we do this check whether there are any pairs of retain releases
208+
// we can safely remove.
209+
tryRemoveRetainReleasePairsBetween(RCIA, UnsafeGuaranteedI,
210+
LastRetainInst, LastRelease,
211+
UnsafeGuaranteedEndI);
212+
157213
LastRetainInst->eraseFromParent();
158214
LastRelease->eraseFromParent();
159215
UnsafeGuaranteedEndI->eraseFromParent();

‎test/SILOptimizer/unsafe_guaranteed_peephole.sil

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,3 +542,85 @@ bb2(%9: $Error):
542542
strong_release %5 : $Foo
543543
throw %9: $Error
544544
}
545+
546+
// CHECK-LABEL: sil @testUnsafeGuaranteed_simple_retain_release_pair
547+
// CHECK: bb0([[P:%.*]] : $Foo):
548+
// CHECK-NOT: retain
549+
// CHECK-NOT: unsafeGuaranteed
550+
// CHECK: [[M:%.*]] = class_method [[P]] : $Foo, #Foo.beep
551+
// CHECK: apply [[M]]([[P]])
552+
// CHECK-NOT: release
553+
// CHECK-NOT: unsafeGuaranteedEnd
554+
// CHECK: [[T:%.*]] = tuple ()
555+
// CHECK: return [[T]]
556+
// CHECK: }
557+
sil @testUnsafeGuaranteed_simple_retain_release_pair : $@convention(method) (@guaranteed Foo) -> () {
558+
bb0(%0 : $Foo):
559+
strong_retain %0 : $Foo
560+
%4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
561+
%5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
562+
%6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
563+
%19 = class_method %5 : $Foo, #Foo.beep!1 : (Foo) -> () -> (), $@convention(method) (@guaranteed Foo) -> ()
564+
retain_value %4 : $(Foo, Builtin.Int8)
565+
%20 = apply %19(%5) : $@convention(method) (@guaranteed Foo) -> ()
566+
strong_release %5 : $Foo
567+
strong_release %5 : $Foo
568+
%16 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
569+
%17 = tuple ()
570+
return %17 : $()
571+
}
572+
573+
// CHECK-LABEL: sil @testUnsafeGuaranteed_simple_retain_release_pair2
574+
// CHECK: bb0([[P:%.*]] : $Foo):
575+
// CHECK-NOT: retain
576+
// CHECK-NOT: unsafeGuaranteed
577+
// CHECK: [[M:%.*]] = class_method [[P]] : $Foo, #Foo.beep
578+
// CHECK: apply [[M]]([[P]])
579+
// CHECK-NOT: release
580+
// CHECK-NOT: unsafeGuaranteedEnd
581+
// CHECK: [[T:%.*]] = tuple ()
582+
// CHECK: return [[T]]
583+
// CHECK: }
584+
sil @testUnsafeGuaranteed_simple_retain_release_pair2 : $@convention(method) (@guaranteed Foo) -> () {
585+
bb0(%0 : $Foo):
586+
strong_retain %0 : $Foo
587+
%4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
588+
%5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
589+
%6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
590+
%19 = class_method %5 : $Foo, #Foo.beep!1 : (Foo) -> () -> (), $@convention(method) (@guaranteed Foo) -> ()
591+
retain_value %4 : $(Foo, Builtin.Int8)
592+
%20 = apply %19(%5) : $@convention(method) (@guaranteed Foo) -> ()
593+
strong_release %5 : $Foo
594+
%16 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
595+
strong_release %5 : $Foo
596+
%17 = tuple ()
597+
return %17 : $()
598+
}
599+
600+
601+
// CHECK-LABEL: sil @testUnsafeGuaranteed_simple_retain_release_pair_not_safe
602+
// CHECK: bb0([[P:%.*]] : $Foo):
603+
// CHECK: [[M:%.*]] = class_method [[P]] : $Foo, #Foo.beep
604+
// CHECK: retain
605+
// CHECK: release
606+
// CHECK: apply [[M]]([[P]])
607+
// CHECK: release
608+
// CHECK: [[T:%.*]] = tuple ()
609+
// CHECK: return [[T]]
610+
// CHECK: }
611+
sil @testUnsafeGuaranteed_simple_retain_release_pair_not_safe : $@convention(method) (@guaranteed Foo) -> () {
612+
bb0(%0 : $Foo):
613+
strong_retain %0 : $Foo
614+
%4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
615+
%5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
616+
%6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
617+
%19 = class_method %5 : $Foo, #Foo.beep!1 : (Foo) -> () -> (), $@convention(method) (@guaranteed Foo) -> ()
618+
retain_value %4 : $(Foo, Builtin.Int8)
619+
strong_release %0 : $Foo
620+
%20 = apply %19(%5) : $@convention(method) (@guaranteed Foo) -> ()
621+
strong_release %5 : $Foo
622+
strong_release %5 : $Foo
623+
%16 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
624+
%17 = tuple ()
625+
return %17 : $()
626+
}

0 commit comments

Comments
 (0)