Skip to content

Commit 77cd7b5

Browse files
eecksteineeckstein
authored
Merge pull request #71554 from eeckstein/fix-escape-utils
EscapeUtils: fix escape result in case an address is stored
2 parents 3b7b451 + abd6ce0 commit 77cd7b5

File tree

4 files changed

+128
-18
lines changed

4 files changed

+128
-18
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,13 @@ struct EscapeUtilityTypes {
239239
///
240240
let followStores: Bool
241241

242+
/// Set to true if an address is stored.
243+
/// This unusual situation can happen if an address is converted to a raw pointer and that pointer
244+
/// is stored to a memory location.
245+
/// In this case the walkers need to follow load instructions even if the visitor and current projection
246+
/// path don't say so.
247+
let addressIsStored: Bool
248+
242249
/// Not nil, if the exact type of the current value is know.
243250
///
244251
/// This is used for destructor analysis.
@@ -251,20 +258,29 @@ struct EscapeUtilityTypes {
251258
let knownType: Type?
252259

253260
func with(projectionPath: SmallProjectionPath) -> Self {
254-
return Self(projectionPath: projectionPath, followStores: self.followStores, knownType: self.knownType)
261+
return Self(projectionPath: projectionPath, followStores: self.followStores,
262+
addressIsStored: self.addressIsStored, knownType: self.knownType)
255263
}
256264

257265
func with(followStores: Bool) -> Self {
258-
return Self(projectionPath: self.projectionPath, followStores: followStores, knownType: self.knownType)
266+
return Self(projectionPath: self.projectionPath, followStores: followStores,
267+
addressIsStored: self.addressIsStored, knownType: self.knownType)
259268
}
260269

270+
func with(addressStored: Bool) -> Self {
271+
return Self(projectionPath: self.projectionPath, followStores: self.followStores, addressIsStored: addressStored,
272+
knownType: self.knownType)
273+
}
274+
261275
func with(knownType: Type?) -> Self {
262-
return Self(projectionPath: self.projectionPath, followStores: self.followStores, knownType: knownType)
276+
return Self(projectionPath: self.projectionPath, followStores: self.followStores,
277+
addressIsStored: self.addressIsStored, knownType: knownType)
263278
}
264279

265280
func merge(with other: EscapePath) -> EscapePath {
266281
let mergedPath = self.projectionPath.merge(with: other.projectionPath)
267282
let mergedFollowStores = self.followStores || other.followStores
283+
let mergedAddrStored = self.addressIsStored || other.addressIsStored
268284
let mergedKnownType: Type?
269285
if let ty = self.knownType {
270286
if let otherTy = other.knownType, ty != otherTy {
@@ -275,7 +291,8 @@ struct EscapeUtilityTypes {
275291
} else {
276292
mergedKnownType = other.knownType
277293
}
278-
return EscapePath(projectionPath: mergedPath, followStores: mergedFollowStores, knownType: mergedKnownType)
294+
return EscapePath(projectionPath: mergedPath, followStores: mergedFollowStores,
295+
addressIsStored: mergedAddrStored, knownType: mergedKnownType)
279296
}
280297
}
281298

@@ -362,7 +379,10 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
362379
}
363380
case is StoreInst, is StoreWeakInst, is StoreUnownedInst:
364381
let store = instruction as! StoringInstruction
365-
assert(operand == store.sourceOperand )
382+
assert(operand == store.sourceOperand)
383+
if !followLoads(at: path) {
384+
return walkUp(address: store.destination, path: path.with(addressStored: true))
385+
}
366386
return walkUp(address: store.destination, path: path)
367387
case is DestroyValueInst, is ReleaseValueInst, is StrongReleaseInst:
368388
if handleDestroy(of: operand.value, path: path) == .abortWalk {
@@ -456,7 +476,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
456476
return walkUp(value: store.source, path: path)
457477
}
458478
case let copyAddr as CopyAddrInst:
459-
if !followLoads(at: path.projectionPath) {
479+
if !followLoads(at: path) {
460480
return .continueWalk
461481
}
462482
if operand == copyAddr.sourceOperand {
@@ -491,7 +511,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
491511
// 2. something can escape in a destructor when the context is destroyed
492512
return walkDownUses(ofValue: pai, path: path.with(knownType: nil))
493513
case is LoadInst, is LoadWeakInst, is LoadUnownedInst, is LoadBorrowInst:
494-
if !followLoads(at: path.projectionPath) {
514+
if !followLoads(at: path) {
495515
return .continueWalk
496516
}
497517
let svi = instruction as! SingleValueInstruction
@@ -561,7 +581,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
561581
}
562582

563583
// Indirect arguments cannot escape the function, but loaded values from such can.
564-
if !followLoads(at: path.projectionPath) &&
584+
if !followLoads(at: path) &&
565585
// Except for begin_apply: it can yield an address value.
566586
!apply.isBeginApplyWithIndirectResults {
567587
return .continueWalk
@@ -622,7 +642,8 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
622642
// Continue at the destination of an arg-to-arg escape.
623643
let arg = argOp.value
624644

625-
let p = Path(projectionPath: toPath, followStores: false, knownType: nil)
645+
let p = Path(projectionPath: toPath, followStores: false, addressIsStored: argPath.addressIsStored,
646+
knownType: nil)
626647
if walkUp(addressOrValue: arg, path: p) == .abortWalk {
627648
return .abortWalk
628649
}
@@ -634,8 +655,9 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
634655
return isEscaping
635656
}
636657

637-
let p = Path(projectionPath: toPath, followStores: false, knownType: exclusive ? argPath.knownType : nil)
638-
658+
let p = Path(projectionPath: toPath, followStores: false, addressIsStored: argPath.addressIsStored,
659+
knownType: exclusive ? argPath.knownType : nil)
660+
639661
if walkDownUses(ofValue: result, path: p) == .abortWalk {
640662
return .abortWalk
641663
}
@@ -700,7 +722,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
700722
case let ap as ApplyInst:
701723
return walkUpApplyResult(apply: ap, path: path.with(knownType: nil))
702724
case is LoadInst, is LoadWeakInst, is LoadUnownedInst, is LoadBorrowInst:
703-
if !followLoads(at: path.projectionPath) {
725+
if !followLoads(at: path) {
704726
// When walking up we shouldn't end up at a load where followLoads is false,
705727
// because going from a (non-followLoads) address to a load always involves a class indirection.
706728
// There is one exception: loading a raw pointer, e.g.
@@ -743,7 +765,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
743765
case is AllocStackInst:
744766
return cachedWalkDown(addressOrValue: def, path: path.with(knownType: nil))
745767
case let arg as FunctionArgument:
746-
if !followLoads(at: path.projectionPath) && arg.convention.isExclusiveIndirect && !path.followStores {
768+
if !followLoads(at: path) && arg.convention.isExclusiveIndirect && !path.followStores {
747769
return cachedWalkDown(addressOrValue: def, path: path.with(knownType: nil))
748770
} else {
749771
return isEscaping
@@ -781,7 +803,8 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
781803
}
782804
let arg = argOp.value
783805

784-
let p = Path(projectionPath: effect.pathPattern, followStores: path.followStores, knownType: nil)
806+
let p = Path(projectionPath: effect.pathPattern, followStores: path.followStores,
807+
addressIsStored: path.addressIsStored, knownType: nil)
785808
if walkUp(addressOrValue: arg, path: p) == .abortWalk {
786809
return .abortWalk
787810
}
@@ -840,10 +863,11 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
840863
return false
841864
}
842865

843-
private func followLoads(at path: SmallProjectionPath) -> Bool {
866+
private func followLoads(at path: Path) -> Bool {
844867
return visitor.followLoads ||
845868
// When part of a class field we have to follow loads.
846-
path.mayHaveClassProjection
869+
path.projectionPath.mayHaveClassProjection ||
870+
path.addressIsStored
847871
}
848872

849873
private func pathForArgumentEscapeChecking(_ path: SmallProjectionPath) -> SmallProjectionPath {
@@ -867,7 +891,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
867891

868892
private extension SmallProjectionPath {
869893
var escapePath: EscapeUtilityTypes.EscapePath {
870-
EscapeUtilityTypes.EscapePath(projectionPath: self, followStores: false, knownType: nil)
894+
EscapeUtilityTypes.EscapePath(projectionPath: self, followStores: false, addressIsStored: false, knownType: nil)
871895
}
872896
}
873897

‎test/SILOptimizer/addr_escape_info.sil

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ sil @inout_class_argument : $@convention(thin) (@inout X) -> () {
4848
}
4949
sil @container_argument : $@convention(thin) (@guaranteed Container) -> ()
5050
sil @take_closure_as_addr : $@convention(thin) (@in @callee_guaranteed () -> ()) -> ()
51+
sil @take_closure_as_addr_noescape : $@convention(thin) (@in @callee_guaranteed () -> ()) -> () {
52+
[%0: noescape **]
53+
}
5154
sil @closure_with_inout : $@convention(thin) (@inout Str) -> () {
5255
[%0: noescape **]
5356
}
@@ -56,6 +59,7 @@ sil @initX : $@convention(method) (@owned X) -> @owned X {
5659
}
5760
sil @modifyStr : $@convention(method) (@inout Str) -> ()
5861
sil @guaranteed_yield_coroutine : $@yield_once @convention(thin) (@inout X) -> @yields @inout X
62+
sil @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
5963

6064
// CHECK-LABEL: Address escape information for test_simple:
6165
// CHECK: value: %1 = struct_element_addr %0 : $*Str, #Str.a
@@ -406,7 +410,7 @@ bb0:
406410
// CHECK-LABEL: Address escape information for partial_apply_with_inout:
407411
// CHECK: value: %0 = alloc_stack $Str
408412
// CHECK-NEXT: ==> %7 = apply %6(%4) : $@convention(thin) (@in @callee_guaranteed () -> ()) -> ()
409-
// CHECK-NEXT: - %9 = apply %8() : $@convention(thin) () -> ()
413+
// CHECK-NEXT: ==> %9 = apply %8() : $@convention(thin) () -> ()
410414
// CHECK: End function partial_apply_with_inout
411415
sil @partial_apply_with_inout : $@convention(thin) () -> () {
412416
bb0:
@@ -426,6 +430,29 @@ bb0:
426430
return %12 : $()
427431
}
428432

433+
// CHECK-LABEL: Address escape information for partial_apply_with_inout_noescape:
434+
// CHECK: value: %0 = alloc_stack $Str
435+
// CHECK-NEXT: ==> %7 = apply %6(%4) : $@convention(thin) (@in @callee_guaranteed () -> ()) -> ()
436+
// CHECK-NEXT: - %9 = apply %8() : $@convention(thin) () -> ()
437+
// CHECK: End function partial_apply_with_inout_noescape
438+
sil @partial_apply_with_inout_noescape : $@convention(thin) () -> () {
439+
bb0:
440+
%0 = alloc_stack $Str
441+
fix_lifetime %0 : $*Str
442+
%2 = function_ref @closure_with_inout : $@convention(thin) (@inout Str) -> ()
443+
%3 = partial_apply [callee_guaranteed] %2(%0) : $@convention(thin) (@inout Str) -> ()
444+
%4 = alloc_stack $@callee_guaranteed () -> ()
445+
store %3 to %4 : $*@callee_guaranteed () -> ()
446+
%6 = function_ref @take_closure_as_addr_noescape : $@convention(thin) (@in @callee_guaranteed () -> ()) -> ()
447+
%7 = apply %6(%4) : $@convention(thin) (@in @callee_guaranteed () -> ()) -> ()
448+
%8 = function_ref @no_arguments : $@convention(thin) () -> ()
449+
%9 = apply %8() : $@convention(thin) () -> ()
450+
dealloc_stack %4 : $*@callee_guaranteed () -> ()
451+
dealloc_stack %0 : $*Str
452+
%12 = tuple ()
453+
return %12 : $()
454+
}
455+
429456
// CHECK-LABEL: Address escape information for non_escaping_trivial_member:
430457
// CHECK: value: %2 = struct $XandInt (%1 : $X, %0 : $Int)
431458
// CHECK-NEXT: - %8 = apply %7(%6) : $@convention(thin) (@in Int) -> ()
@@ -709,3 +736,22 @@ bb0(%0 : $Int):
709736
return %r : $()
710737
}
711738

739+
// CHECK-LABEL: Address escape information for test_stored_pointer:
740+
// CHECK: value: %0 = alloc_stack $Int
741+
// CHECK-NEXT: ==> %5 = apply %4(%2) : $@convention(thin) (@in Builtin.RawPointer) -> ()
742+
// CHECK: End function test_stored_pointer
743+
sil @test_stored_pointer : $@convention(thin) () -> () {
744+
bb0:
745+
%0 = alloc_stack $Int
746+
%1 = address_to_pointer %0 : $*Int to $Builtin.RawPointer
747+
%2 = alloc_stack $Builtin.RawPointer
748+
store %1 to %2 : $*Builtin.RawPointer
749+
%4 = function_ref @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
750+
%5 = apply %4(%2) : $@convention(thin) (@in Builtin.RawPointer) -> ()
751+
dealloc_stack %2 : $*Builtin.RawPointer
752+
fix_lifetime %0 : $*Int
753+
dealloc_stack %0 : $*Int
754+
%8 = tuple ()
755+
return %8 : $()
756+
}
757+

‎test/SILOptimizer/mem-behavior.sil

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ sil @single_indirect_arg_coroutine : $@yield_once @convention(thin) (@in Int32)
3030
sil @indirect_arg_and_ptr : $@convention(thin) (@in Int32, Builtin.RawPointer) -> Int32
3131
sil @single_reference : $@convention(thin) (@guaranteed X) -> Int32
3232
sil @nouser_func : $@convention(thin) () -> ()
33+
sil @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
3334

3435
sil @store_to_int : $@convention(thin) (Int32, @inout Int32) -> () {
3536
[%1: write v**]
@@ -1098,3 +1099,22 @@ bb0:
10981099
%4 = tuple ()
10991100
return %4 : $()
11001101
}
1102+
1103+
// CHECK-LABEL: @test_stored_pointer
1104+
// CHECK: PAIR #3.
1105+
// CHECK-NEXT: %5 = apply %4(%2) : $@convention(thin) (@in Builtin.RawPointer) -> ()
1106+
// CHECK-NEXT: %0 = alloc_stack $Int // users: %7, %1
1107+
// CHECK-NEXT: r=1,w=1
1108+
sil @test_stored_pointer : $@convention(thin) () -> () {
1109+
bb0:
1110+
%0 = alloc_stack $Int
1111+
%1 = address_to_pointer %0 : $*Int to $Builtin.RawPointer
1112+
%2 = alloc_stack $Builtin.RawPointer
1113+
store %1 to %2 : $*Builtin.RawPointer
1114+
%4 = function_ref @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
1115+
%5 = apply %4(%2) : $@convention(thin) (@in Builtin.RawPointer) -> ()
1116+
dealloc_stack %2 : $*Builtin.RawPointer
1117+
dealloc_stack %0 : $*Int
1118+
%8 = tuple ()
1119+
return %8 : $()
1120+
}

‎test/SILOptimizer/redundant_load_elim.sil

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ sil @use_twofield : $@convention(thin) (TwoField) -> ()
134134
sil @escaped_a_ptr : $@convention(thin) () -> @out A
135135
sil @escaped_a : $@convention(thin) () -> Builtin.RawPointer
136136
sil @init_twofield : $@convention(thin) (@thin TwoField.Type) -> TwoField
137+
sil @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
137138

138139

139140
// We have a bug in the old projection code which this test case exposes.
@@ -1295,3 +1296,22 @@ bb0:
12951296
%7 = tuple (%4 : $String, %5 : $Int64)
12961297
return %7 : $(String, Int64)
12971298
}
1299+
1300+
// CHECK-LABEL: sil @dont_remove_load_from_stored_pointer :
1301+
// CHECK: [[L:%[0-9]+]] = load
1302+
// CHECK: return [[L]]
1303+
// CHECK-LABEL: } // end sil function 'dont_remove_load_from_stored_pointer'
1304+
sil @dont_remove_load_from_stored_pointer : $@convention(thin) (Int) -> Int {
1305+
bb0(%0 : $Int):
1306+
%1 = alloc_stack $Int
1307+
store %0 to %1 : $*Int
1308+
%5 = address_to_pointer %1 : $*Int to $Builtin.RawPointer
1309+
%32 = alloc_stack $Builtin.RawPointer
1310+
store %5 to %32 : $*Builtin.RawPointer
1311+
%34 = function_ref @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
1312+
%35 = apply %34(%32) : $@convention(thin) (@in Builtin.RawPointer) -> ()
1313+
dealloc_stack %32 : $*Builtin.RawPointer
1314+
%53 = load %1 : $*Int
1315+
dealloc_stack %1 : $*Int
1316+
return %53 : $Int
1317+
}

0 commit comments

Comments
 (0)