Skip to content

Commit

Permalink
Merge pull request #73610 from eeckstein/fix-compute-side-effects-6.0
Browse files Browse the repository at this point in the history
[6.0] ComputeSideEffects: correctly handle escaping arguments
  • Loading branch information
eeckstein committed May 14, 2024
2 parents c9e109e + 618e8ee commit a17d360
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ private struct CollectedEffects {
}

mutating func addEffectsForEscapingArgument(argument: FunctionArgument) {
var escapeWalker = ArgumentEscapingWalker()
var escapeWalker = ArgumentEscapingWalker(context)

if escapeWalker.hasUnknownUses(argument: argument) {
// Worst case: we don't know anything about how the argument escapes.
Expand Down Expand Up @@ -414,13 +414,18 @@ private func defaultPath(for value: Value) -> SmallProjectionPath {
/// Checks if an argument escapes to some unknown user.
private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
var walkDownCache = WalkerCache<UnusedWalkingPath>()
private let calleeAnalysis: CalleeAnalysis

/// True if the argument escapes to a load which (potentially) "takes" the memory location.
private(set) var foundTakingLoad = false

/// True, if the argument escapes to a closure context which might be destroyed when called.
private(set) var foundConsumingPartialApply = false

init(_ context: FunctionPassContext) {
self.calleeAnalysis = context.calleeAnalysis
}

mutating func hasUnknownUses(argument: FunctionArgument) -> Bool {
if argument.type.isAddress {
return walkDownUses(ofAddress: argument, path: UnusedWalkingPath()) == .abortWalk
Expand All @@ -444,14 +449,23 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
return .continueWalk

case let apply as ApplySite:
if apply.isCallee(operand: value) {
// `CollectedEffects.handleApply` only handles argument operands of an apply, but not the callee operand.
return .abortWalk
}
if let pa = apply as? PartialApplyInst, !pa.isOnStack {
foundConsumingPartialApply = true
}
return .continueWalk
// `CollectedEffects.handleApply` only handles argument operands of an apply, but not the callee operand.
if let calleeArgIdx = apply.calleeArgumentIndex(of: value),
let callees = calleeAnalysis.getCallees(callee: apply.callee)
{
// If an argument escapes in a called function, we don't know anything about the argument's side effects.
// For example, it could escape to the return value and effects might occur in the caller.
for callee in callees {
if callee.effects.escapeEffects.canEscape(argumentIndex: calleeArgIdx, path: SmallProjectionPath.init(.anyValueFields)) {
return .abortWalk
}
}
return .continueWalk
}
return .abortWalk
default:
return .abortWalk
}
Expand Down
45 changes: 42 additions & 3 deletions test/SILOptimizer/side_effects.sil
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ sil @$s4test1ZCfD : $@convention(method) (@owned Z) -> () {
// CHECK-LABEL: sil @load_store_to_args
// CHECK-NEXT: [%0: read v**]
// CHECK-NEXT: [%1: write v**]
// CHECK-NEXT: [%2: write c0.v**]
// CHECK-NEXT: [%2: noescape **, write c0.v**]
// CHECK-NEXT: [global: ]
// CHECK-NEXT: {{^[^[]}}
sil @load_store_to_args : $@convention(thin) (@inout Int32, @inout Int32, @guaranteed X) -> () {
[%2: noescape **]
bb0(%0 : $*Int32, %1 : $*Int32, %2 : $X):
%l1 = load %0 : $*Int32
store %l1 to %1 : $*Int32
Expand Down Expand Up @@ -971,11 +972,13 @@ bb0(%0 : @guaranteed $S):
}

// CHECK-LABEL: sil [ossa] @storeToArgs
// CHECK-NEXT: [%0: write c0.v**]
// CHECK-NEXT: [%1: write c0.v**]
// CHECK-NEXT: [%0: noescape **, write c0.v**]
// CHECK-NEXT: [%1: noescape **, write c0.v**]
// CHECK-NEXT: [global: ]
// CHECK-NEXT: {{^[^[]}}
sil [ossa] @storeToArgs : $@convention(thin) (@guaranteed List, @guaranteed List) -> () {
[%0: noescape **]
[%1: noescape **]
bb0(%1 : @guaranteed $List, %2 : @guaranteed $List):
cond_br undef, bb1, bb2

Expand Down Expand Up @@ -1195,3 +1198,39 @@ bb0(%0 : $*T):
%r = tuple()
return %r : $()
}

sil @store_owned_to_out : $@convention(thin) (@owned SP) -> @out SP {
[%0: noescape **, write v**]
[%1: escape v** -> %0.v**]
[global: ]
}

// CHECK-LABEL: sil @test_escaping_arg1
// CHECK-NEXT: [%0: write v**.c*.v**, copy v**.c*.v**, destroy v**.c*.v**]
// CHECK-NEXT: [global: write,copy,destroy]
// CHECK-NEXT: {{^[^[]}}
sil @test_escaping_arg1 : $@convention(thin) (@owned SP) -> () {
bb0(%0 : $SP):
%1 = alloc_stack $SP
%2 = function_ref @store_owned_to_out : $@convention(thin) (@owned SP) -> @out SP
%3 = apply %2(%1, %0) : $@convention(thin) (@owned SP) -> @out SP
%4 = struct_element_addr %1 : $*SP, #SP.value
%5 = load %4 : $*X
strong_release %5 : $X
dealloc_stack %1 : $*SP
%r = tuple ()
return %r : $()
}

// CHECK-LABEL: sil @test_escaping_arg2
// CHECK-NEXT: [%0: read v**.c*.v**, write v**.c*.v**, copy v**.c*.v**, destroy v**.c*.v**]
// CHECK-NEXT: [global: read,write,copy,destroy,allocate,deinit_barrier]
// CHECK-NEXT: {{^[^[]}}
sil @test_escaping_arg2 : $@convention(thin) (@owned SP) -> () {
bb0(%0 : $SP):
%1 = function_ref @forward_to_return : $@convention(thin) (@owned SP) -> @owned SP
%2 = apply %1(%0) : $@convention(thin) (@owned SP) -> @owned SP
release_value %2 : $SP
%r = tuple ()
return %r : $()
}

0 comments on commit a17d360

Please sign in to comment.