Skip to content

Commit 6010cfb

Browse files
eecksteineeckstein
authored
Merge pull request #77852 from eeckstein/fix-aliasinfo
AliasAnalysis: fix memory-behavior of closures with inout arguments
2 parents c690fef + 53bb72c commit 6010cfb

File tree

4 files changed

+138
-1
lines changed

4 files changed

+138
-1
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Analysis/AliasAnalysis.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,12 @@ private struct FullApplyEffectsVisitor : EscapeVisitorWithResult {
741741
return .ignore
742742
}
743743
if user == apply {
744+
if apply.isCallee(operand: operand) {
745+
// If the address "escapes" to the callee of the apply it means that the address was captured
746+
// by an inout_aliasable operand of an partial_apply.
747+
// Therefore assume that the called function will both, read and write, to the address.
748+
return .abort
749+
}
744750
let e = calleeAnalysis.getSideEffects(of: apply, operand: operand, path: path.projectionPath)
745751
result.merge(with: e)
746752
}

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,9 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
546546
// We need to follow the partial_apply value for two reasons:
547547
// 1. the closure (with the captured values) itself can escape
548548
// 2. something can escape in a destructor when the context is destroyed
549-
return walkDownUses(ofValue: pai, path: path.with(knownType: nil))
549+
if followLoads(at: path) || pai.capturesAddress(of: operand) {
550+
return walkDownUses(ofValue: pai, path: path.with(knownType: nil))
551+
}
550552
case is LoadInst, is LoadWeakInst, is LoadUnownedInst, is LoadBorrowInst:
551553
if !followLoads(at: path) {
552554
return .continueWalk
@@ -966,3 +968,23 @@ private extension SmallProjectionPath {
966968
EscapeUtilityTypes.EscapePath(projectionPath: self, followStores: false, addressIsStored: false, knownType: nil)
967969
}
968970
}
971+
972+
private extension PartialApplyInst {
973+
func capturesAddress(of operand: Operand) -> Bool {
974+
assert(operand.value.type.isAddress)
975+
guard let conv = convention(of: operand) else {
976+
fatalError("callee operand of partial_apply cannot have address type")
977+
}
978+
switch conv {
979+
case .indirectIn, .indirectInGuaranteed:
980+
// A partial_apply copies the values from indirect-in arguments, but does not capture the address.
981+
return false
982+
case .indirectInout, .indirectInoutAliasable, .packInout:
983+
return true
984+
case .directOwned, .directUnowned, .directGuaranteed, .packOwned, .packGuaranteed:
985+
fatalError("invalid convention for address operand")
986+
case .indirectOut, .packOut, .indirectInCXX:
987+
fatalError("invalid convention for partial_apply")
988+
}
989+
}
990+
}

‎test/SILOptimizer/dead_store_elim.sil

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,3 +1692,21 @@ bb0(%0 : $Int, %1 : $Builtin.Int64, %2 : $Builtin.RawPointer):
16921692
return %r : $()
16931693
}
16941694

1695+
sil @closure_with_inout : $@convention(thin) (@inout_aliasable Int) -> ()
1696+
1697+
// CHECK-LABEL: sil @closure_with_inout_reads_argument :
1698+
// CHECK: store %0
1699+
// CHECK: end sil function 'closure_with_inout_reads_argument'
1700+
sil @closure_with_inout_reads_argument : $@convention(thin) (Int) -> () {
1701+
bb0(%0 : $Int):
1702+
%1 = alloc_stack $Int
1703+
store %0 to %1
1704+
%3 = function_ref @closure_with_inout : $@convention(thin) (@inout_aliasable Int) -> ()
1705+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%1) : $@convention(thin) (@inout_aliasable Int) -> ()
1706+
%50 = apply %4() : $@noescape @callee_guaranteed () -> ()
1707+
dealloc_stack %4 : $@noescape @callee_guaranteed () -> ()
1708+
dealloc_stack %1 : $*Int
1709+
%r = tuple ()
1710+
return %r
1711+
}
1712+

‎test/SILOptimizer/mem-behavior.sil

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,6 +1656,97 @@ bb0(%0 : $*X):
16561656
return %3 : $()
16571657
}
16581658

1659+
// CHECK-LABEL: @test_non_escaping_applied
1660+
// CHECK: PAIR #0.
1661+
// CHECK-NEXT: %2 = partial_apply [on_stack] %1(%0) : $@convention(thin) (@in_guaranteed X) -> ()
1662+
// CHECK-NEXT: %0 = argument of bb0 : $*X
1663+
// CHECK-NEXT: r=1,w=0
1664+
// CHECK: PAIR #1.
1665+
// CHECK-NEXT: %3 = apply %2() : $@noescape @callee_owned () -> ()
1666+
// CHECK-NEXT: %0 = argument of bb0 : $*X
1667+
// CHECK-NEXT: r=0,w=0
1668+
sil [ossa] @test_non_escaping_applied : $@convention(thin) (@in X) -> () {
1669+
bb0(%0 : $*X):
1670+
%1 = function_ref @indirect_X : $@convention(thin) (@in_guaranteed X) -> ()
1671+
%2 = partial_apply [on_stack] %1(%0) : $@convention(thin) (@in_guaranteed X) -> ()
1672+
%3 = apply %2() : $@noescape @callee_owned () -> ()
1673+
destroy_addr %0
1674+
%6 = tuple ()
1675+
return %6 : $()
1676+
}
1677+
1678+
sil @closure : $@convention(thin) (@inout_aliasable Int) -> ()
1679+
sil @closure2 : $@convention(thin) (Int, @inout_aliasable Int) -> ()
1680+
1681+
// CHECK-LABEL: @closure_with_inout
1682+
// CHECK: PAIR #0.
1683+
// CHECK-NEXT: %2 = partial_apply [callee_guaranteed] [on_stack] %1(%0) : $@convention(thin) (@inout_aliasable Int) -> ()
1684+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1685+
// CHECK-NEXT: r=0,w=0
1686+
// CHECK: PAIR #1.
1687+
// CHECK-NEXT: %3 = apply %2() : $@noescape @callee_guaranteed () -> ()
1688+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1689+
// CHECK-NEXT: r=1,w=1
1690+
sil @closure_with_inout : $@convention(thin) (@inout Int) -> () {
1691+
bb0(%0 : $*Int):
1692+
%1 = function_ref @closure : $@convention(thin) (@inout_aliasable Int) -> ()
1693+
%2 = partial_apply [callee_guaranteed] [on_stack] %1(%0) : $@convention(thin) (@inout_aliasable Int) -> ()
1694+
%3 = apply %2() : $@noescape @callee_guaranteed () -> ()
1695+
dealloc_stack %2
1696+
%r = tuple ()
1697+
return %r
1698+
}
1699+
1700+
// CHECK-LABEL: @two_closures_with_inout
1701+
// CHECK: PAIR #0.
1702+
// CHECK-NEXT: %3 = partial_apply [callee_guaranteed] [on_stack] %2(%0) : $@convention(thin) (Int, @inout_aliasable Int) -> ()
1703+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1704+
// CHECK-NEXT: r=0,w=0
1705+
// CHECK: PAIR #1.
1706+
// CHECK-NEXT: %4 = partial_apply [callee_guaranteed] [on_stack] %3(%1) : $@noescape @callee_guaranteed (Int) -> ()
1707+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1708+
// CHECK-NEXT: r=0,w=0
1709+
// CHECK: PAIR #2.
1710+
// CHECK-NEXT: %5 = apply %4() : $@noescape @callee_guaranteed () -> ()
1711+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1712+
// CHECK-NEXT: r=1,w=1
1713+
sil @two_closures_with_inout : $@convention(thin) (@inout Int, Int) -> () {
1714+
bb0(%0 : $*Int, %1 : $Int):
1715+
%2 = function_ref @closure2 : $@convention(thin) (Int, @inout_aliasable Int) -> ()
1716+
%3 = partial_apply [callee_guaranteed] [on_stack] %2(%0) : $@convention(thin) (Int, @inout_aliasable Int) -> ()
1717+
%4 = partial_apply [callee_guaranteed] [on_stack] %3(%1) : $@noescape @callee_guaranteed (Int) -> ()
1718+
%5 = apply %4() : $@noescape @callee_guaranteed () -> ()
1719+
dealloc_stack %4
1720+
dealloc_stack %3
1721+
%r = tuple ()
1722+
return %r
1723+
}
1724+
1725+
sil @call_closure : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> () {
1726+
[%0: read v**.c*.v**, write v**.c*.v**]
1727+
[global: read,write]
1728+
}
1729+
1730+
// CHECK-LABEL: @pass_closure_to_function
1731+
// CHECK: PAIR #0.
1732+
// CHECK-NEXT: %2 = partial_apply [callee_guaranteed] [on_stack] %1(%0) : $@convention(thin) (@inout_aliasable Int) -> ()
1733+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1734+
// CHECK-NEXT: r=0,w=0
1735+
// CHECK: PAIR #1.
1736+
// CHECK-NEXT: %4 = apply %3(%2) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
1737+
// CHECK-NEXT: %0 = argument of bb0 : $*Int
1738+
// CHECK-NEXT: r=1,w=1
1739+
sil @pass_closure_to_function : $@convention(thin) (@inout Int) -> () {
1740+
bb0(%0 : $*Int):
1741+
%4 = function_ref @closure : $@convention(thin) (@inout_aliasable Int) -> ()
1742+
%5 = partial_apply [callee_guaranteed] [on_stack] %4(%0) : $@convention(thin) (@inout_aliasable Int) -> ()
1743+
%6 = function_ref @call_closure : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
1744+
%7 = apply %6(%5) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
1745+
dealloc_stack %5
1746+
%r = tuple ()
1747+
return %r
1748+
}
1749+
16591750
// CHECK-LABEL: @test_is_unique
16601751
// CHECK: PAIR #0.
16611752
// CHECK-NEXT: %2 = is_unique %0 : $*X

0 commit comments

Comments
 (0)