Skip to content

Commit fba1f1d

Browse files
eecksteineeckstein
authored
Merge pull request #81253 from eeckstein/copy-block-opt2
Optimizer: support partial_apply of thunks in the `copy_block` simplification
2 parents 29416b1 + 1fee4fe commit fba1f1d

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
@@ -29,7 +29,7 @@ extension CopyBlockInst : Simplifiable, SILCombineSimplifiable {
2929
///
3030
func simplify(_ context: SimplifyContext) {
3131
if hasValidUses(block: self) {
32-
replaceBlock( self, with: operand.value, context)
32+
replaceBlock(self, with: operand.value, context)
3333
context.erase(instruction: self)
3434
}
3535
}
@@ -44,6 +44,13 @@ private func hasValidUses(block: Value) -> Bool {
4444
}
4545
case let apply as FullApplySite where apply.isCallee(operand: use):
4646
break
47+
case let partialApply as PartialApplyInst:
48+
// If the block is passed to another function - either as closure argument or as closure capture -
49+
// it's "converted" to a swift closure with the help of a thunk. The thunk just calls the block.
50+
// If this is a non-escaping partial_apply and it's such a thunk, the block does not escape.
51+
if partialApply.canClosureArgumentEscape(closure: use) {
52+
return false
53+
}
4754
case is EndBorrowInst, is DestroyValueInst:
4855
break
4956
default:
@@ -61,10 +68,40 @@ private func replaceBlock(_ block: Value, with original: Value, _ context: Simpl
6168
context.erase(instruction: beginBorrow)
6269
case is FullApplySite:
6370
use.set(to: original, context)
71+
case let partialApply as PartialApplyInst:
72+
if original.ownership == .unowned {
73+
let builder = Builder(before: partialApply, context)
74+
let conv = builder.createUncheckedOwnershipConversion(operand: original, resultOwnership: .guaranteed)
75+
use.set(to: conv, context)
76+
} else {
77+
use.set(to: original, context)
78+
}
6479
case is EndBorrowInst, is DestroyValueInst:
6580
context.erase(instruction: use.instruction)
6681
default:
6782
fatalError("unhandled use")
6883
}
6984
}
7085
}
86+
87+
private extension PartialApplyInst {
88+
func canClosureArgumentEscape(closure: Operand) -> Bool {
89+
guard isOnStack,
90+
let callee = referencedFunction,
91+
callee.isDefinition,
92+
let argIdx = calleeArgumentIndex(of: closure),
93+
// If the callee only _calls_ the closure argument, it does not escape.
94+
callee.arguments[argIdx].uses.allSatisfy(isCalleeOperandOfApply)
95+
else {
96+
return true
97+
}
98+
return false
99+
}
100+
}
101+
102+
private func isCalleeOperandOfApply(_ operand: Operand) -> Bool {
103+
if let apply = operand.instruction as? FullApplySite, apply.isCallee(operand: operand) {
104+
return true
105+
}
106+
return false
107+
}

‎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
@@ -1178,6 +1178,8 @@ struct BridgedBuilder{
11781178
BridgedInstruction::CastingIsolatedConformances isolatedConformances,
11791179
BridgedValue source, BridgedCanType sourceFormalType,
11801180
BridgedValue destination, BridgedCanType targetFormalType) const;
1181+
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createUncheckedOwnershipConversion(
1182+
BridgedValue op, BridgedValue::Ownership ownership) const;
11811183
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createLoad(BridgedValue op, SwiftInt ownership) const;
11821184
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createLoadBorrow(BridgedValue op) const;
11831185
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
@@ -2205,6 +2205,13 @@ BridgedInstruction BridgedBuilder::createLoad(BridgedValue op, SwiftInt ownershi
22052205
(swift::LoadOwnershipQualifier)ownership)};
22062206
}
22072207

2208+
2209+
BridgedInstruction BridgedBuilder::createUncheckedOwnershipConversion(BridgedValue op,
2210+
BridgedValue::Ownership ownership) const {
2211+
return {unbridged().createUncheckedOwnershipConversion(regularLoc(), op.getSILValue(),
2212+
BridgedValue::unbridge(ownership))};
2213+
}
2214+
22082215
BridgedInstruction BridgedBuilder::createLoadBorrow(BridgedValue op) const {
22092216
return {unbridged().createLoadBorrow(regularLoc(), op.getSILValue())};
22102217
}

‎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
@@ -17,7 +17,9 @@ module CModule {
1717
@import Foundation;
1818

1919
@interface TestClass : NSObject
20-
- (void)callHandlerInline: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
20+
- (void)callHandler1: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
21+
- (void)callHandler2: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
22+
- (void)callHandler3: (NS_NOESCAPE _Nonnull dispatch_block_t)block;
2123
@end
2224

2325

@@ -27,13 +29,34 @@ import CModule
2729

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

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

‎test/SILOptimizer/simplify_copy_block.sil

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

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

0 commit comments

Comments
 (0)