Skip to content

Commit a0dc560

Browse files
authored
Merge pull request #70559 from eeckstein/fix-simplify-begin-borrow
SimplifyBeginBorrow: fix `isDestroyed(after:)`
2 parents df858a5 + 1669788 commit a0dc560

File tree

2 files changed

+94
-2
lines changed

2 files changed

+94
-2
lines changed

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyBeginBorrow.swift

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,28 @@ private extension Value {
118118
}
119119

120120
func isDestroyed(after nonDestroyUser: Instruction) -> Bool {
121-
uses.getSingleUser(notOfType: DestroyValueInst.self) == nonDestroyUser
121+
return uses.getSingleUser(notOfType: DestroyValueInst.self) == nonDestroyUser &&
122+
nonDestroyUser.dominates(destroysOf: self)
122123
}
123124

124125
func replaceAllDestroys(with replacement: Value, _ context: SimplifyContext) {
125126
uses.filterUsers(ofType: DestroyValueInst.self).replaceAll(with: replacement, context)
126127
}
127128
}
128129

130+
private extension Instruction {
131+
func dominates(destroysOf value: Value) -> Bool {
132+
// In instruction simplification we don't have a domtree. Therefore do a simple dominance
133+
// check based on same-block relations.
134+
if parentBlock == value.parentBlock {
135+
// The value and instruction are in the same block. All uses are dominanted by both.
136+
return true
137+
}
138+
let destroys = value.uses.filterUsers(ofType: DestroyValueInst.self)
139+
return destroys.allSatisfy({ $0.instruction.parentBlock == parentBlock})
140+
}
141+
}
142+
129143
private extension ForwardingInstruction {
130144
func isSingleForwardedOperand(_ operand: Operand) -> Bool {
131145
switch self {

test/SILOptimizer/simplify_begin_borrow.sil

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import Builtin
88
import Swift
99
import SwiftShims
1010

11-
class C {
11+
class B {}
12+
class C : B {
1213
@_hasStorage let i: Int
1314
}
1415

@@ -360,3 +361,80 @@ bb3:
360361
return %0 : $C
361362
}
362363

364+
// CHECK-LABEL: sil [ossa] @dont_replace_destroys_of_original_with_destroys_of_nondominating_forward : {{.*}} {
365+
// CHECK: begin_borrow
366+
// CHECK-LABEL: } // end sil function 'dont_replace_destroys_of_original_with_destroys_of_nondominating_forward'
367+
sil [ossa] @dont_replace_destroys_of_original_with_destroys_of_nondominating_forward : $@convention(thin) (@owned C) -> () {
368+
bb0(%21 : @owned $C):
369+
cond_br undef, bb3, bb6
370+
371+
bb3:
372+
%39 = begin_borrow %21 : $C
373+
%40 = upcast %39 : $C to $B
374+
apply undef(%40) : $@convention(thin) (@guaranteed B) -> ()
375+
end_borrow %39 : $C
376+
cond_br undef, bb4, bb5
377+
378+
bb4:
379+
destroy_value %21 : $C
380+
br bb7
381+
382+
bb5:
383+
destroy_value %21 : $C
384+
br bb7
385+
386+
bb6:
387+
destroy_value %21 : $C
388+
br bb7
389+
390+
bb7:
391+
%retval = tuple ()
392+
return %retval : $()
393+
}
394+
395+
// CHECK-LABEL: sil [ossa] @do_replace_destroys_of_original_with_destroys_of_dominating_forward : {{.*}} {
396+
// CHECK-NOT: begin_borrow
397+
// CHECK-LABEL: } // end sil function 'do_replace_destroys_of_original_with_destroys_of_dominating_forward'
398+
sil [ossa] @do_replace_destroys_of_original_with_destroys_of_dominating_forward : $@convention(thin) (@owned C) -> () {
399+
bb0(%21 : @owned $C):
400+
%39 = begin_borrow %21 : $C
401+
%40 = upcast %39 : $C to $B
402+
apply undef(%40) : $@convention(thin) (@guaranteed B) -> ()
403+
end_borrow %39 : $C
404+
cond_br undef, bb4, bb5
405+
406+
bb4:
407+
destroy_value %21 : $C
408+
br bb7
409+
410+
bb5:
411+
destroy_value %21 : $C
412+
br bb7
413+
414+
bb7:
415+
%retval = tuple ()
416+
return %retval : $()
417+
}
418+
419+
// CHECK-LABEL: sil [ossa] @do_replace_destroys_of_original_with_destroy_in_same_block : {{.*}} {
420+
// CHECK-NOT: begin_borrow
421+
// CHECK-LABEL: } // end sil function 'do_replace_destroys_of_original_with_destroy_in_same_block'
422+
sil [ossa] @do_replace_destroys_of_original_with_destroy_in_same_block : $@convention(thin) (@owned C) -> () {
423+
bb0(%0 : @owned $C):
424+
cond_br undef, bb1, bb2
425+
426+
bb1:
427+
br bb3
428+
429+
bb2:
430+
br bb3
431+
432+
bb3:
433+
%4 = begin_borrow %0 : $C
434+
%5 = upcast %4 : $C to $B
435+
apply undef(%5) : $@convention(thin) (@guaranteed B) -> ()
436+
end_borrow %4 : $C
437+
destroy_value %0 : $C
438+
%retval = tuple ()
439+
return %retval : $()
440+
}

0 commit comments

Comments
 (0)