Skip to content

Commit cd684a3

Browse files
committed
Bailout of DestroyHoisting if there were no hoistable destroys
Otherwise, DestroyHoisting attemps to compute interior liveness and inserts destoys at the ends. CanonicalizeOSSALifetimes is designed to do this and not DestroyHoisting.
1 parent 6c2d3eb commit cd684a3

File tree

2 files changed

+56
-4
lines changed

2 files changed

+56
-4
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/DestroyHoisting.swift

+9-3
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,13 @@ private func optimize(value: Value, _ context: FunctionPassContext) {
8888
return
8989
}
9090

91-
var hoistableDestroys = selectHoistableDestroys(of: value, context)
91+
var (foundDestroys, hoistableDestroys) = selectHoistableDestroys(of: value, context)
9292
defer { hoistableDestroys.deinitialize() }
9393

94+
guard foundDestroys else {
95+
return
96+
}
97+
9498
guard var minimalLiverange = InstructionRange(withLiverangeOf: value, ignoring: hoistableDestroys, context) else {
9599
return
96100
}
@@ -99,12 +103,13 @@ private func optimize(value: Value, _ context: FunctionPassContext) {
99103
hoistDestroys(of: value, toEndOf: minimalLiverange, restrictingTo: &hoistableDestroys, context)
100104
}
101105

102-
private func selectHoistableDestroys(of value: Value, _ context: FunctionPassContext) -> InstructionSet {
106+
private func selectHoistableDestroys(of value: Value, _ context: FunctionPassContext) -> (Bool, InstructionSet) {
103107
// Also includes liveranges of copied values and values stored to memory.
104108
var forwardExtendedLiverange = InstructionRange(withForwardExtendedLiverangeOf: value, context)
105109
defer { forwardExtendedLiverange.deinitialize() }
106110

107111
let deadEndBlocks = context.deadEndBlocks
112+
var foundDestroys = false
108113
var hoistableDestroys = InstructionSet(context)
109114

110115
for use in value.uses {
@@ -114,10 +119,11 @@ private func selectHoistableDestroys(of value: Value, _ context: FunctionPassCon
114119
// TODO: once we have complete OSSA lifetimes we don't need to handle dead-end blocks.
115120
!deadEndBlocks.isDeadEnd(destroy.parentBlock)
116121
{
122+
foundDestroys = true
117123
hoistableDestroys.insert(destroy)
118124
}
119125
}
120-
return hoistableDestroys
126+
return (foundDestroys, hoistableDestroys)
121127
}
122128

123129
private func hoistDestroys(of value: Value,

test/SILOptimizer/rdar146142041.sil

+47-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all %s -semantic-arc-opts | %FileCheck %s
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -semantic-arc-opts -destroy-hoisting | %FileCheck %s
22

33
import Swift
44

@@ -13,6 +13,7 @@ struct NonTrivial {
1313

1414
sil @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
1515

16+
// test1 is the reduction from rdar://146142041. test2 and test3 are mutations of test1 and may not be created from swift code in practice
1617
// CHECK-LABEL: sil [ossa] @test1 :
1718
// CHECK: [[PA:%.*]] = partial_apply
1819
// CHECK: [[COPY:%.*]] = copy_value [[PA]]
@@ -111,3 +112,48 @@ bb8:
111112
return %24
112113
}
113114

115+
// Ensure no ownership verification error
116+
sil [ossa] @test3 : $@convention(thin) () -> () {
117+
bb0:
118+
cond_br undef, bb2, bb1
119+
120+
bb1:
121+
%1 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.none!enumelt
122+
br bb7(%1)
123+
124+
bb2:
125+
br bb3
126+
127+
bb3:
128+
%4 = alloc_box ${ var Optional<NonTrivial> }
129+
%5 = begin_borrow [lexical] %4
130+
%6 = project_box %5, 0
131+
inject_enum_addr %6, #Optional.none!enumelt
132+
cond_br undef, bb4, bb5
133+
134+
bb4:
135+
unreachable
136+
137+
bb5:
138+
br bb6
139+
140+
bb6:
141+
%11 = function_ref @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
142+
%12 = partial_apply [callee_guaranteed] %11(%6) : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
143+
%13 = copy_value %12
144+
%14 = convert_escape_to_noescape %13 to $@noescape @callee_guaranteed () -> ()
145+
%15 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt, %13
146+
end_borrow %5
147+
destroy_value %4
148+
destroy_value %12
149+
destroy_value %14
150+
br bb7(%15)
151+
152+
bb7(%21 : @owned $Optional<@callee_guaranteed () -> ()>):
153+
destroy_value %21
154+
br bb8
155+
156+
bb8:
157+
%24 = tuple ()
158+
return %24
159+
}

0 commit comments

Comments
 (0)