Skip to content

Commit 11b77bf

Browse files
eecksteineeckstein
authored
Merge pull request #81328 from eeckstein/copy-block-opt2-6.2
[6.2] Optimizer: support partial_apply of thunks in the `copy_block` simplification
2 parents 751678d + cc92319 commit 11b77bf

File tree

7 files changed

+203
-5
lines changed

7 files changed

+203
-5
lines changed

‎SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyCopyBlock.swift‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ extension CopyBlockInst : Simplifiable, SILCombineSimplifiable {
3434
}
3535

3636
if hasValidUses(block: self) {
37-
replaceBlock( self, with: operand.value, context)
37+
replaceBlock(self, with: operand.value, context)
3838
context.erase(instruction: self)
3939
}
4040
}
@@ -49,6 +49,13 @@ private func hasValidUses(block: Value) -> Bool {
4949
}
5050
case let apply as FullApplySite where apply.isCallee(operand: use):
5151
break
52+
case let partialApply as PartialApplyInst:
53+
// If the block is passed to another function - either as closure argument or as closure capture -
54+
// it's "converted" to a swift closure with the help of a thunk. The thunk just calls the block.
55+
// If this is a non-escaping partial_apply and it's such a thunk, the block does not escape.
56+
if partialApply.canClosureArgumentEscape(closure: use) {
57+
return false
58+
}
5259
case is EndBorrowInst, is DestroyValueInst:
5360
break
5461
default:
@@ -66,10 +73,40 @@ private func replaceBlock(_ block: Value, with original: Value, _ context: Simpl
6673
context.erase(instruction: beginBorrow)
6774
case is FullApplySite:
6875
use.set(to: original, context)
76+
case let partialApply as PartialApplyInst:
77+
if original.ownership == .unowned {
78+
let builder = Builder(before: partialApply, context)
79+
let conv = builder.createUncheckedOwnershipConversion(operand: original, resultOwnership: .guaranteed)
80+
use.set(to: conv, context)
81+
} else {
82+
use.set(to: original, context)
83+
}
6984
case is EndBorrowInst, is DestroyValueInst:
7085
context.erase(instruction: use.instruction)
7186
default:
7287
fatalError("unhandled use")
7388
}
7489
}
7590
}
91+
92+
private extension PartialApplyInst {
93+
func canClosureArgumentEscape(closure: Operand) -> Bool {
94+
guard isOnStack,
95+
let callee = referencedFunction,
96+
callee.isDefinition,
97+
let argIdx = calleeArgumentIndex(of: closure),
98+
// If the callee only _calls_ the closure argument, it does not escape.
99+
callee.arguments[argIdx].uses.allSatisfy(isCalleeOperandOfApply)
100+
else {
101+
return true
102+
}
103+
return false
104+
}
105+
}
106+
107+
private func isCalleeOperandOfApply(_ operand: Operand) -> Bool {
108+
if let apply = operand.instruction as? FullApplySite, apply.isCallee(operand: operand) {
109+
return true
110+
}
111+
return false
112+
}

‎SwiftCompilerSources/Sources/SIL/Builder.swift‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,13 @@ public struct Builder {
215215
return notifyNew(cast.getAs(UnconditionalCheckedCastAddrInst.self))
216216
}
217217

218+
public func createUncheckedOwnershipConversion(
219+
operand: Value, resultOwnership: Ownership
220+
) -> UncheckedOwnershipConversionInst {
221+
let uoc = bridged.createUncheckedOwnershipConversion(operand.bridged, resultOwnership._bridged)
222+
return notifyNew(uoc.getAs(UncheckedOwnershipConversionInst.self))
223+
}
224+
218225
public func createLoad(fromAddress: Value, ownership: LoadInst.LoadOwnership) -> LoadInst {
219226
let load = bridged.createLoad(fromAddress.bridged, ownership.rawValue)
220227
return notifyNew(load.getAs(LoadInst.self))

‎include/swift/SIL/SILBridging.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,6 +1172,8 @@ struct BridgedBuilder{
11721172
BridgedInstruction::CastingIsolatedConformances isolatedConformances,
11731173
BridgedValue source, BridgedCanType sourceFormalType,
11741174
BridgedValue destination, BridgedCanType targetFormalType) const;
1175+
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createUncheckedOwnershipConversion(
1176+
BridgedValue op, BridgedValue::Ownership ownership) const;
11751177
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createLoad(BridgedValue op, SwiftInt ownership) const;
11761178
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createLoadBorrow(BridgedValue op) const;
11771179
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createBeginDeallocRef(BridgedValue reference,

‎include/swift/SIL/SILBridgingImpl.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2190,6 +2190,13 @@ BridgedInstruction BridgedBuilder::createLoad(BridgedValue op, SwiftInt ownershi
21902190
(swift::LoadOwnershipQualifier)ownership)};
21912191
}
21922192

2193+
2194+
BridgedInstruction BridgedBuilder::createUncheckedOwnershipConversion(BridgedValue op,
2195+
BridgedValue::Ownership ownership) const {
2196+
return {unbridged().createUncheckedOwnershipConversion(regularLoc(), op.getSILValue(),
2197+
BridgedValue::unbridge(ownership))};
2198+
}
2199+
21932200
BridgedInstruction BridgedBuilder::createLoadBorrow(BridgedValue op) const {
21942201
return {unbridged().createLoadBorrow(regularLoc(), op.getSILValue())};
21952202
}

‎lib/SIL/Verifier/SILOwnershipVerifier.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,15 @@ bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses(
658658
}
659659
}
660660

661+
if (auto *uoc = dyn_cast<UncheckedOwnershipConversionInst>(value)) {
662+
// When converting from `unowned`, which can happen for ObjectiveC blocks,
663+
// we don't require and scope-ending instruction. This falls in the category:
664+
// "trust me, I know what I'm doing".
665+
if (uoc->getOperand()->getOwnershipKind() == OwnershipKind::Unowned) {
666+
return true;
667+
}
668+
}
669+
661670
// If we have an unowned value, then again there is nothing left to do.
662671
if (value->getOwnershipKind() == OwnershipKind::Unowned)
663672
return true;

‎test/SILOptimizer/optimize_copy_block.swift‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ module CModule {
1818
@import Foundation;
1919

2020
@interface TestClass : NSObject
21-
- (void)callHandlerInline: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
21+
- (void)callHandler1: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
22+
- (void)callHandler2: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
23+
- (void)callHandler3: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
2224
@end
2325

2426

@@ -28,13 +30,34 @@ import CModule
2830

2931
@objc @implementation
3032
extension TestClass {
31-
// CHECK-LABEL: sil private [thunk] @$sSo9TestClassC4testE17callHandlerInlineyyyyXEFTo :
33+
// CHECK-LABEL: sil private [thunk] @$sSo9TestClassC4testE12callHandler1yyyyXEFTo :
3234
// CHECK-NOT: copy_block
3335
// CHECK: apply %0
3436
// CHECK-NOT: destroy_value
35-
// CHECK: } // end sil function '$sSo9TestClassC4testE17callHandlerInlineyyyyXEFTo'
36-
func callHandlerInline(_ handler: () -> Void) {
37+
// CHECK: } // end sil function '$sSo9TestClassC4testE12callHandler1yyyyXEFTo'
38+
func callHandler1(_ handler: () -> Void) {
3739
handler()
3840
}
41+
42+
// CHECK-LABEL: sil private [thunk] @$sSo9TestClassC4testE12callHandler2yyyyXEFTo :
43+
// CHECK-NOT: copy_block
44+
// CHECK: } // end sil function '$sSo9TestClassC4testE12callHandler2yyyyXEFTo'
45+
func callHandler2(_ handler: () -> Void) {
46+
callee(handler)
47+
}
48+
49+
// CHECK-LABEL: sil private [thunk] @$sSo9TestClassC4testE12callHandler3yyyyXEFTo :
50+
// CHECK-NOT: copy_block
51+
// CHECK: } // end sil function '$sSo9TestClassC4testE12callHandler3yyyyXEFTo'
52+
func callHandler3(_ handler: () -> Void) {
53+
callClosure {
54+
handler()
55+
}
56+
}
3957
}
4058

59+
@_silgen_name("callee")
60+
func callee(_ handler: () -> Void)
61+
62+
@_silgen_name("callClosure")
63+
func callClosure(_ closure: () -> ())

‎test/SILOptimizer/simplify_copy_block.sil‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,119 @@ bb0(%0 : @unowned $@convention(block) @noescape () -> ()):
2727
return %7
2828
}
2929

30+
// CHECK-LABEL: sil [ossa] @remove_copy_block_with_partial_apply :
31+
// ENABLED-NOT: copy_block
32+
// ENABLED: [[B:%.*]] = unchecked_ownership_conversion %0, @unowned to @guaranteed
33+
// ENABLED: [[PA:%.*]] = partial_apply [callee_guaranteed] [on_stack] {{%[0-9]+}}([[B]])
34+
// ENABLED: destroy_value [[PA]]
35+
// ENABLED-NOT: destroy_value
36+
// CHECK: } // end sil function 'remove_copy_block_with_partial_apply'
37+
sil [ossa] @remove_copy_block_with_partial_apply : $@convention(thin) (@convention(block) @noescape () -> ()) -> () {
38+
bb0(%0 : @unowned $@convention(block) @noescape () -> ()):
39+
%2 = copy_block %0
40+
%3 = function_ref @thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
41+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%2) : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
42+
destroy_value %4
43+
destroy_value %2
44+
%7 = tuple ()
45+
return %7
46+
}
47+
48+
// CHECK-LABEL: sil [ossa] @remove_copy_block_with_partial_apply_guaranteed :
49+
// ENABLED-NOT: copy_block
50+
// ENABLED: [[PA:%.*]] = partial_apply [callee_guaranteed] [on_stack] {{%[0-9]+}}(%0)
51+
// ENABLED: destroy_value [[PA]]
52+
// ENABLED-NOT: destroy_value
53+
// CHECK: } // end sil function 'remove_copy_block_with_partial_apply_guaranteed'
54+
sil [ossa] @remove_copy_block_with_partial_apply_guaranteed : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> () {
55+
bb0(%0 : @guaranteed $@convention(block) @noescape () -> ()):
56+
%2 = copy_block %0
57+
%3 = function_ref @thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
58+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%2) : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
59+
destroy_value %4
60+
destroy_value %2
61+
%7 = tuple ()
62+
return %7
63+
}
64+
65+
// CHECK-LABEL: sil [ossa] @remove_copy_block_with_partial_apply_owned :
66+
// ENABLED-NOT: copy_block
67+
// ENABLED: [[PA:%.*]] = partial_apply [callee_guaranteed] [on_stack] {{%[0-9]+}}(%0)
68+
// CHECK: } // end sil function 'remove_copy_block_with_partial_apply_owned'
69+
sil [ossa] @remove_copy_block_with_partial_apply_owned : $@convention(thin) (@owned @convention(block) @noescape () -> ()) -> () {
70+
bb0(%0 : @owned $@convention(block) @noescape () -> ()):
71+
%2 = copy_block %0
72+
%3 = function_ref @thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
73+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%2) : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
74+
destroy_value %4
75+
destroy_value %2
76+
destroy_value %0
77+
%7 = tuple ()
78+
return %7
79+
}
80+
81+
sil [ossa] @thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> () {
82+
bb0(%0 : @guaranteed $@convention(block) @noescape () -> ()):
83+
%1 = apply %0() : $@convention(block) @noescape () -> ()
84+
%2 = tuple ()
85+
return %2
86+
}
87+
88+
sil @use_closure : $@convention(thin) (@guaranteed @callee_guaranteed () -> ()) -> ()
89+
90+
// CHECK-LABEL: sil [ossa] @dont_remove_copy_block_with_escaping_partial_apply :
91+
// CHECK: copy_block
92+
// CHECK: } // end sil function 'dont_remove_copy_block_with_escaping_partial_apply'
93+
sil [ossa] @dont_remove_copy_block_with_escaping_partial_apply : $@convention(thin) (@convention(block) @noescape () -> ()) -> () {
94+
bb0(%0 : @unowned $@convention(block) @noescape () -> ()):
95+
%2 = copy_block %0
96+
%3 = function_ref @thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
97+
%4 = partial_apply [callee_guaranteed] %3(%2) : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
98+
%5 = function_ref @use_closure : $@convention(thin) (@guaranteed @callee_guaranteed () -> ()) -> ()
99+
apply %5(%4) : $@convention(thin) (@guaranteed @callee_guaranteed () -> ()) -> ()
100+
destroy_value %4
101+
%7 = tuple ()
102+
return %7
103+
}
104+
105+
sil [ossa] @unknown_thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
106+
107+
// CHECK-LABEL: sil [ossa] @dont_remove_copy_block_with_unknown_partial_apply :
108+
// CHECK: copy_block
109+
// CHECK: } // end sil function 'dont_remove_copy_block_with_unknown_partial_apply'
110+
sil [ossa] @dont_remove_copy_block_with_unknown_partial_apply : $@convention(thin) (@convention(block) @noescape () -> ()) -> () {
111+
bb0(%0 : @unowned $@convention(block) @noescape () -> ()):
112+
%2 = copy_block %0
113+
%3 = function_ref @unknown_thunk : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
114+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%2) : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
115+
destroy_value %4
116+
destroy_value %2
117+
%7 = tuple ()
118+
return %7
119+
}
120+
121+
// CHECK-LABEL: sil [ossa] @dont_remove_copy_block_with_partial_apply_escaping :
122+
// CHECK: copy_block
123+
// CHECK: } // end sil function 'dont_remove_copy_block_with_partial_apply_escaping'
124+
sil [ossa] @dont_remove_copy_block_with_partial_apply_escaping : $@convention(thin) (@convention(block) @noescape () -> ()) -> () {
125+
bb0(%0 : @unowned $@convention(block) @noescape () -> ()):
126+
%2 = copy_block %0
127+
%3 = function_ref @thunk_escaping : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
128+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%2) : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> ()
129+
destroy_value %4
130+
destroy_value %2
131+
%7 = tuple ()
132+
return %7
133+
}
134+
135+
sil [ossa] @thunk_escaping : $@convention(thin) (@guaranteed @convention(block) @noescape () -> ()) -> () {
136+
bb0(%0 : @guaranteed $@convention(block) @noescape () -> ()):
137+
%1 = function_ref @use_block : $@convention(thin) (@convention(block) @noescape () -> ()) -> ()
138+
%2 = apply %1(%0) : $@convention(thin) (@convention(block) @noescape () -> ()) -> ()
139+
%3 = tuple ()
140+
return %3
141+
}
142+
30143
// CHECK-LABEL: sil [ossa] @dont_remove_copied_block :
31144
// CHECK: copy_block
32145
// CHECK: } // end sil function 'dont_remove_copied_block'

0 commit comments

Comments
 (0)