Skip to content

Commit 4088933

Browse files
eecksteineeckstein
committed
ReleaseDevirtualizer: use the ValueUseDefWalker instead of the manual use-def walking implementation
1 parent 70536fb commit 4088933

File tree

2 files changed

+136
-141
lines changed

2 files changed

+136
-141
lines changed

‎SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ReleaseDevirtualizer.swift

Lines changed: 47 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,11 @@ private func tryDevirtualizeReleaseOfObject(
7474
_ deallocStackRef: DeallocStackRefInst
7575
) {
7676
let allocRefInstruction = deallocStackRef.allocRef
77-
var root = release.operands[0].value
78-
while let newRoot = stripRCIdentityPreservingInsts(root) {
79-
root = newRoot
80-
}
8177

82-
if root != allocRefInstruction {
78+
// Check if the release instruction right before the `dealloc_stack_ref` really releases
79+
// the allocated object (and not something else).
80+
var finder = FindAllocationOfRelease(allocation: allocRefInstruction)
81+
if !finder.allocationIsRoot(of: release.operand) {
8382
return
8483
}
8584

@@ -109,119 +108,58 @@ private func tryDevirtualizeReleaseOfObject(
109108
context.erase(instruction: release)
110109
}
111110

112-
private func stripRCIdentityPreservingInsts(_ value: Value) -> Value? {
113-
guard let inst = value as? Instruction else { return nil }
114-
115-
switch inst {
116-
// First strip off RC identity preserving casts.
117-
case is UpcastInst,
118-
is UncheckedRefCastInst,
119-
is InitExistentialRefInst,
120-
is OpenExistentialRefInst,
121-
is RefToBridgeObjectInst,
122-
is BridgeObjectToRefInst,
123-
is ConvertFunctionInst,
124-
is UncheckedEnumDataInst:
125-
return inst.operands[0].value
126-
127-
// Then if we have a struct_extract that is extracting a non-trivial member
128-
// from a struct with no other non-trivial members, a ref count operation on
129-
// the struct is equivalent to a ref count operation on the extracted
130-
// member. Strip off the extract.
131-
case let sei as StructExtractInst where sei.isFieldOnlyNonTrivialField:
132-
return sei.operand
133-
134-
// If we have a struct or tuple instruction with only one non-trivial operand, the
135-
// only reference count that can be modified is the non-trivial operand. Return
136-
// the non-trivial operand.
137-
case is StructInst, is TupleInst:
138-
return inst.uniqueNonTrivialOperand
139-
140-
// If we have an enum instruction with a payload, strip off the enum to
141-
// expose the enum's payload.
142-
case let ei as EnumInst where !ei.operands.isEmpty:
143-
return ei.operand
144-
145-
// If we have a tuple_extract that is extracting the only non trivial member
146-
// of a tuple, a retain_value on the tuple is equivalent to a retain_value on
147-
// the extracted value.
148-
case let tei as TupleExtractInst where tei.isEltOnlyNonTrivialElt:
149-
return tei.operand
150-
151-
default:
152-
return nil
153-
}
154-
}
111+
// Up-walker to find the root of a release instruction.
112+
private struct FindAllocationOfRelease : ValueUseDefWalker {
113+
private let allocInst: AllocRefInstBase
114+
private var allocFound = false
155115

156-
private extension Instruction {
157-
/// Search the operands of this tuple for a unique non-trivial elt. If we find
158-
/// it, return it. Otherwise return `nil`.
159-
var uniqueNonTrivialOperand: Value? {
160-
var candidateElt: Value?
161-
let function = self.function
162-
163-
for op in operands {
164-
if !op.value.type.isTrivial(in: function) {
165-
if candidateElt == nil {
166-
candidateElt = op.value
167-
continue
168-
}
116+
var walkUpCache = WalkerCache<SmallProjectionPath>()
169117

170-
// Otherwise, we have two values that are non-trivial. Bail.
171-
return nil
172-
}
173-
}
118+
init(allocation: AllocRefInstBase) { allocInst = allocation }
174119

175-
return candidateElt
120+
/// The top-level entry point: returns true if the root of `value` is the `allocInst`.
121+
mutating func allocationIsRoot(of value: Value) -> Bool {
122+
let path = SmallProjectionPath().push(.anyValueFields)
123+
return walkUp(value: value, path: path) != .abortWalk &&
124+
allocFound
176125
}
177-
}
178126

179-
private extension TupleExtractInst {
180-
var isEltOnlyNonTrivialElt: Bool {
181-
let function = self.function
182-
183-
if type.isTrivial(in: function) {
184-
return false
127+
mutating func rootDef(value: Value, path: SmallProjectionPath) -> WalkResult {
128+
if value == allocInst {
129+
allocFound = true
130+
return .continueWalk
185131
}
186-
187-
let opType = operand.type
188-
189-
var nonTrivialEltsCount = 0
190-
for elt in opType.tupleElements {
191-
if elt.isTrivial(in: function) {
192-
nonTrivialEltsCount += 1
193-
}
194-
195-
if nonTrivialEltsCount > 1 {
196-
return false
197-
}
198-
}
199-
200-
return true
132+
return .abortWalk
201133
}
202-
}
203-
204-
private extension StructExtractInst {
205-
var isFieldOnlyNonTrivialField: Bool {
206-
let function = self.function
207-
208-
if type.isTrivial(in: function) {
209-
return false
210-
}
211134

212-
let structType = operand.type
213-
214-
var nonTrivialFieldsCount = 0
215-
for field in structType.getNominalFields(in: function) {
216-
if field.isTrivial(in: function) {
217-
nonTrivialFieldsCount += 1
218-
}
219-
220-
if nonTrivialFieldsCount > 1 {
221-
return false
135+
// This function is called for `struct` and `tuple` instructions in case the `path` doesn't select
136+
// a specific operand but all operands.
137+
mutating func walkUpAllOperands(of def: Instruction, path: SmallProjectionPath) -> WalkResult {
138+
139+
// Instead of walking up _all_ operands (which would be the default behavior), require that only a
140+
// _single_ operand is not trivial and walk up that operand.
141+
// We need to check this because the released value must not contain multiple copies of the
142+
// allocated object. We can only replace a _single_ release with the destructor call and not
143+
// multiple releases of the same object. E.g.
144+
//
145+
// %x = alloc_ref [stack] $X
146+
// strong_retain %x
147+
// %t = tuple (%x, %x)
148+
// release_value %t // -> releases %x two times!
149+
// dealloc_stack_ref %x
150+
//
151+
var nonTrivialOperandFound = false
152+
for operand in def.operands {
153+
if !operand.value.type.isTrivial(in: def.function) {
154+
if nonTrivialOperandFound {
155+
return .abortWalk
156+
}
157+
nonTrivialOperandFound = true
158+
if walkUp(value: operand.value, path: path) == .abortWalk {
159+
return .abortWalk
160+
}
222161
}
223162
}
224-
225-
return true
163+
return .continueWalk
226164
}
227165
}

‎test/SILOptimizer/devirt_release.sil

Lines changed: 89 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,12 @@ import Builtin
1111
import Swift
1212

1313
class B {
14+
@_hasStorage var next: B
1415
}
1516

17+
struct Str {
18+
@_hasStorage var b: B
19+
}
1620

1721
// CHECK-LABEL: sil @devirtualize_object
1822
// CHECK: [[A:%[0-9]+]] = alloc_ref
@@ -21,7 +25,7 @@ class B {
2125
// CHECK: [[D:%[0-9]+]] = function_ref @$s4test1BCfD
2226
// CHECK-NEXT: apply [[D]]([[A]])
2327
// CHECK-NEXT: dealloc_stack_ref [[A]]
24-
// CHECK: return
28+
// CHECK: } // end sil function 'devirtualize_object'
2529
sil @devirtualize_object : $@convention(thin) () -> () {
2630
bb0:
2731
%1 = alloc_ref [stack] $B
@@ -36,6 +40,7 @@ bb0:
3640
// CHECK-NEXT: strong_release
3741
// CHECK-NEXT: dealloc_ref
3842
// CHECK: return
43+
// CHECK: } // end sil function 'dont_devirtualize_heap_allocated'
3944
sil @dont_devirtualize_heap_allocated : $@convention(thin) () -> () {
4045
bb0:
4146
%1 = alloc_ref $B
@@ -72,6 +77,7 @@ bb0:
7277
// CHECK-NEXT: strong_release
7378
// CHECK-NEXT: dealloc_ref
7479
// CHECK: return
80+
// CHECK: } // end sil function 'dont_devirtualize_wrong_release'
7581
sil @dont_devirtualize_wrong_release : $@convention(thin) (@owned B) -> () {
7682
bb0(%0 : $B):
7783
%1 = alloc_ref $B
@@ -88,7 +94,7 @@ bb0(%0 : $B):
8894
// CHECK: [[F:%[0-9]+]] = function_ref @unknown_func
8995
// CHECK-NEXT: apply [[F]]()
9096
// CHECK-NEXT: dealloc_ref
91-
// CHECK: return
97+
// CHECK: } // end sil function 'dont_devirtualize_unknown_release'
9298
sil @dont_devirtualize_unknown_release : $@convention(thin) (@owned B) -> () {
9399
bb0(%0 : $B):
94100
%1 = alloc_ref $B
@@ -107,7 +113,7 @@ bb0(%0 : $B):
107113
// CHECK: [[D:%[0-9]+]] = function_ref @$s4test1BCfD
108114
// CHECK-NEXT: apply [[D]]([[A]])
109115
// CHECK-NEXT: dealloc_stack_ref [[A]]
110-
// CHECK: return
116+
// CHECK: } // end sil function 'dont_crash_with_missing_release'
111117
sil @dont_crash_with_missing_release : $@convention(thin) () -> () {
112118
bb0:
113119
%1 = alloc_ref [stack] $B
@@ -119,42 +125,93 @@ bb0:
119125
return %r : $()
120126
}
121127

128+
// CHECK-LABEL: sil @long_chain_from_alloc_to_release
129+
// CHECK: set_deallocating
130+
// CHECK-NOT: release_value
131+
// CHECK: } // end sil function 'long_chain_from_alloc_to_release'
132+
sil @long_chain_from_alloc_to_release : $@convention(thin) () -> () {
133+
bb0:
134+
%1 = alloc_ref [stack] $B
135+
%2 = struct $Str(%1 : $B)
136+
%3 = struct_extract %2 : $Str, #Str.b
137+
cond_br undef, bb1, bb2
138+
139+
bb1:
140+
br bb3(%3 : $B)
141+
142+
bb2:
143+
br bb3(%3 : $B)
144+
145+
bb3(%a : $B):
146+
%s = struct $Str (%a : $B)
147+
%i = integer_literal $Builtin.Int64, 0
148+
%t = tuple (%s : $Str, %i : $Builtin.Int64)
149+
%o = enum $Optional<(Str, Builtin.Int64)>, #Optional.some!enumelt, %t : $(Str, Builtin.Int64)
150+
release_value %o : $Optional<(Str, Builtin.Int64)>
151+
dealloc_stack_ref %1 : $B
152+
%r = tuple ()
153+
return %r : $()
154+
}
122155

123-
sil @unknown_func : $@convention(thin) () -> ()
124-
125-
// test.B.__deallocating_deinit
126-
sil hidden @$s4test1BCfD : $@convention(method) (@owned B) -> () {
127-
// %0 // users: %1, %3
128-
bb0(%0 : $B):
129-
debug_value %0 : $B, let, name "self" // id: %1
130-
// function_ref test.B.deinit
131-
%2 = function_ref @$s4test1BCfd : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject // user: %3
132-
%3 = apply %2(%0) : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject // user: %4
133-
%4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $B // user: %5
134-
dealloc_ref %4 : $B // id: %5
135-
%6 = tuple () // user: %7
136-
return %6 : $() // id: %7
156+
// CHECK-LABEL: sil @dont_devirtualize_double_release
157+
// CHECK-NOT: set_deallocating
158+
// CHECK: release_value
159+
// CHECK: } // end sil function 'dont_devirtualize_double_release'
160+
sil @dont_devirtualize_double_release : $@convention(thin) () -> () {
161+
bb0:
162+
%1 = alloc_ref [stack] $B
163+
%t = tuple (%1 : $B, %1 : $B)
164+
release_value %t : $(B, B)
165+
dealloc_stack_ref %1 : $B
166+
%r = tuple ()
167+
return %r : $()
137168
}
138169

139-
// test.B.deinit
140-
sil hidden @$s4test1BCfd : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject {
141-
// %0 // users: %1, %2
142-
bb0(%0 : $B):
143-
debug_value %0 : $B, let, name "self" // id: %1
144-
%2 = unchecked_ref_cast %0 : $B to $Builtin.NativeObject // user: %3
145-
return %2 : $Builtin.NativeObject // id: %3
170+
// CHECK-LABEL: sil @dont_devirtualize_release_of_load
171+
// CHECK-NOT: set_deallocating
172+
// CHECK: strong_release
173+
// CHECK: } // end sil function 'dont_devirtualize_release_of_load'
174+
sil @dont_devirtualize_release_of_load : $@convention(thin) () -> () {
175+
bb0:
176+
%1 = alloc_ref [stack] $B
177+
%2 = ref_element_addr %1 : $B, #B.next
178+
%3 = load %2 : $*B
179+
strong_release %3 : $B
180+
dealloc_stack_ref %1 : $B
181+
%r = tuple ()
182+
return %r : $()
146183
}
147184

148-
// test.B.init () -> test.B
149-
sil hidden @$s4test1BCACycfc : $@convention(method) (@owned B) -> @owned B {
150-
// %0 // users: %1, %2
151-
bb0(%0 : $B):
152-
debug_value %0 : $B, let, name "self" // id: %1
153-
return %0 : $B // id: %2
185+
// CHECK-LABEL: sil @dont_devirtualize_release_of_different_allocations
186+
// CHECK-NOT: set_deallocating
187+
// CHECK: strong_release
188+
// CHECK: } // end sil function 'dont_devirtualize_release_of_different_allocations'
189+
sil @dont_devirtualize_release_of_different_allocations : $@convention(thin) () -> () {
190+
bb0:
191+
%1 = alloc_ref [stack] $B
192+
%2 = alloc_ref [stack] $B
193+
cond_br undef, bb1, bb2
194+
195+
bb1:
196+
br bb3(%1 : $B)
197+
198+
bb2:
199+
br bb3(%2 : $B)
200+
201+
bb3(%a : $B):
202+
strong_release %a : $B
203+
dealloc_stack_ref %2 : $B
204+
dealloc_stack_ref %1 : $B
205+
%r = tuple ()
206+
return %r : $()
154207
}
155208

209+
210+
sil @unknown_func : $@convention(thin) () -> ()
211+
212+
sil @$s4test1BCfD : $@convention(method) (@owned B) -> ()
213+
156214
sil_vtable B {
157-
#B.deinit!deallocator: @$s4test1BCfD // test.B.__deallocating_deinit
158-
#B.init!initializer: @$s4test1BCACycfc // test.B.init () -> test.B
215+
#B.deinit!deallocator: @$s4test1BCfD
159216
}
160217

0 commit comments

Comments
 (0)