Skip to content

Commit 113e23d

Browse files
committed
ComputeEscapeEffects: correctly handle "exclusive" argument -> return effects
* Disallow stores in the return -> argument path. When walking up in the EscapeUtils, it's allowed to follow stores. Therefore stores wouldn't be handled correctly. * Also make sure that there is a return -> argument path at all Fixes a wrong address-escaping effect in case the called function copies an indirect argument to a newly created object. rdar://105133434
1 parent caea41a commit 113e23d

File tree

3 files changed

+69
-7
lines changed

3 files changed

+69
-7
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,25 +197,39 @@ private func isOperandOfRecursiveCall(_ op: Operand) -> Bool {
197197
return false
198198
}
199199

200-
/// Returns true if when walking from the `toSelection` to the `fromArgument`,
201-
/// there are no other arguments or escape points than `fromArgument`. Also, the
202-
/// path at the `fromArgument` must match with `fromPath`.
200+
/// Returns true if when walking up from the `returnInst`, the `fromArgument` is the one
201+
/// and only argument which is reached - with a matching `fromPath`.
203202
private
204203
func isExclusiveEscapeToReturn(fromArgument: Argument, fromPath: SmallProjectionPath,
205204
toPath: SmallProjectionPath,
206205
returnInst: ReturnInst, _ context: FunctionPassContext) -> Bool {
207-
struct IsExclusiveReturnEscapeVisitor : EscapeVisitor {
206+
struct IsExclusiveReturnEscapeVisitor : EscapeVisitorWithResult {
208207
let fromArgument: Argument
209208
let fromPath: SmallProjectionPath
210209
let toPath: SmallProjectionPath
210+
var result = false
211211

212212
mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult {
213-
if operand.instruction is ReturnInst {
213+
switch operand.instruction {
214+
case is ReturnInst:
214215
if path.followStores { return .abort }
215216
if path.projectionPath.matches(pattern: toPath) {
216217
return .ignore
217218
}
218219
return .abort
220+
case let si as StoringInstruction:
221+
// Don't allow store instructions because this allows the EscapeUtils to walk up
222+
// an apply result with `followStores`.
223+
if operand == si.destinationOperand {
224+
return .abort
225+
}
226+
case let ca as CopyAddrInst:
227+
// `copy_addr` is like a store.
228+
if operand == ca.destinationOperand {
229+
return .abort
230+
}
231+
default:
232+
break
219233
}
220234
return .continueWalk
221235
}
@@ -226,11 +240,12 @@ func isExclusiveEscapeToReturn(fromArgument: Argument, fromPath: SmallProjection
226240
}
227241
if path.followStores { return .abort }
228242
if arg == fromArgument && path.projectionPath.matches(pattern: fromPath) {
243+
result = true
229244
return .walkDown
230245
}
231246
return .abort
232247
}
233248
}
234249
let visitor = IsExclusiveReturnEscapeVisitor(fromArgument: fromArgument, fromPath: fromPath, toPath: toPath)
235-
return !returnInst.operand.at(toPath).isEscaping(using: visitor, context)
250+
return returnInst.operand.at(toPath).visit(using: visitor, context) ?? false
236251
}

test/SILOptimizer/escape_effects.sil

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ bb0(%0 : $*Z, %1 : $Y):
331331
}
332332

333333
// CHECK-LABEL: sil @store_to_content
334-
// CHECK-NEXT: [%0: escape => %r, escape c*.v** => %r.c*.v**]
334+
// CHECK-NEXT: [%0: escape => %r, escape c*.v** -> %r.c*.v**]
335335
// CHECK-NEXT: {{^[^[]}}
336336
sil @store_to_content : $@convention(thin) (@owned Z, @guaranteed Y) -> @owned Z {
337337
bb0(%0 : $Z, %1 : $Y):
@@ -372,3 +372,15 @@ bb0(%0 : $*Z, %1 : $Y):
372372
%5 = tuple ()
373373
return %5 : $()
374374
}
375+
376+
// CHECK-LABEL: sil @store_to_class_field
377+
// CHECK-NEXT: [%0: escape v** -> %r.c0.v**, escape v**.c*.v** -> %r.c0.v**.c*.v**]
378+
// CHECK-NEXT: {{^[^[]}}
379+
sil @store_to_class_field : $@convention(thin) (@in_guaranteed Str) -> @owned Y {
380+
bb0(%0 : $*Str):
381+
%1 = alloc_ref $Y
382+
%2 = ref_element_addr %1 : $Y, #Y.s
383+
copy_addr %0 to [init] %2 : $*Str
384+
return %1 : $Y
385+
}
386+
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %target-swift-frontend -O -module-name=test -emit-sil %s | %FileCheck %s
2+
3+
protocol P {}
4+
5+
public struct S {
6+
let p: P // force S to be an address-only type
7+
let i: Int
8+
}
9+
10+
public final class X {
11+
let s: S
12+
13+
init(s: S) {
14+
self.s = s
15+
}
16+
}
17+
18+
@inline(never)
19+
func createX(s: S) -> X {
20+
return X(s: s)
21+
}
22+
23+
// Test that the load is a barrier for release hoisting and that the strong_release is not hoisted above the load.
24+
25+
// CHECK-LABEL: sil @$s4test6testit1sSiAA1SV_tF
26+
// CHECK: [[X:%[0-9]+]] = apply
27+
// CHECK: [[R:%[0-9]+]] = load
28+
// CHECK: strong_release [[X]]
29+
// CHECK: return [[R]]
30+
// CHECK: } // end sil function '$s4test6testit1sSiAA1SV_tF'
31+
public func testit(s: S) -> Int {
32+
let x = createX(s: s)
33+
return x.s.i
34+
}
35+

0 commit comments

Comments
 (0)