Skip to content

Commit a4c675e

Browse files
committed
CopyToBorrowOptimization: fix insertion order of end_borrows
In liferange exit blocks the end_borrows were inserted in the wrong order. Fixes an ownership verification error rdar://142632741
1 parent ce3cc37 commit a4c675e

File tree

2 files changed

+39
-12
lines changed

2 files changed

+39
-12
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CopyToBorrowOptimization.swift

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ private struct Uses {
122122
// Exit blocks of the load/copy_value's liverange which don't have a destroy.
123123
// Those are successor blocks of terminators, like `switch_enum`, which do _not_ forward the value.
124124
// E.g. the none-case of a switch_enum of an Optional.
125-
private(set) var nonDestroyingLiverangeExits: Stack<BasicBlock>
125+
private(set) var nonDestroyingLiverangeExits: Stack<Instruction>
126+
127+
var allLifetimeEndingInstructions: [Instruction] {
128+
Array(destroys.lazy.map { $0 }) + Array(nonDestroyingLiverangeExits)
129+
}
126130

127131
private(set) var usersInDeadEndBlocks: Stack<Instruction>
128132

@@ -179,7 +183,7 @@ private struct Uses {
179183
// A terminator instruction can implicitly end the lifetime of its operand in a success block,
180184
// e.g. a `switch_enum` with a non-payload case block. Such success blocks need an `end_borrow`, though.
181185
for succ in termInst.successors where !succ.arguments.contains(where: {$0.ownership == .owned}) {
182-
nonDestroyingLiverangeExits.append(succ)
186+
nonDestroyingLiverangeExits.append(succ.instructions.first!)
183187
}
184188
}
185189
}
@@ -268,7 +272,7 @@ private func remove(copy: CopyValueInst, collectedUses: Uses, liverange: Instruc
268272
context.erase(instructions: collectedUses.destroys)
269273
}
270274

271-
// Handle the special case if the `load` or `copy_valuw` is immediately followed by a single `move_value`.
275+
// Handle the special case if the `load` or `copy_value` is immediately followed by a single `move_value`.
272276
// In this case we have to preserve the move's flags by inserting a `begin_borrow` with the same flags.
273277
// For example:
274278
//
@@ -314,15 +318,11 @@ private func createEndBorrows(for beginBorrow: Value, atEndOf liverange: Instruc
314318
// destroy_value %2
315319
// destroy_value %3 // The final destroy. Here we need to create the `end_borrow`(s)
316320
//
317-
for destroy in collectedUses.destroys where !liverange.contains(destroy) {
318-
let builder = Builder(before: destroy, context)
319-
builder.createEndBorrow(of: beginBorrow)
320-
}
321-
for liverangeExitBlock in collectedUses.nonDestroyingLiverangeExits where
322-
!liverange.blockRange.contains(liverangeExitBlock)
323-
{
324-
let builder = Builder(atBeginOf: liverangeExitBlock, context)
325-
builder.createEndBorrow(of: beginBorrow)
321+
for endInst in collectedUses.allLifetimeEndingInstructions {
322+
if !liverange.contains(endInst) {
323+
let builder = Builder(before: endInst, context)
324+
builder.createEndBorrow(of: beginBorrow)
325+
}
326326
}
327327
}
328328

test/SILOptimizer/copy-to-borrow-optimization.sil

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,3 +2119,30 @@ bb0(%0 : @owned $Klass):
21192119
unreachable
21202120
}
21212121

2122+
// CHECK-LABEL: sil [ossa] @end_borrows_in_liferange_exit_block :
2123+
// CHECK: %1 = load_borrow %0
2124+
// CHECK: %2 = begin_borrow [lexical] %1
2125+
// CHECK: bb1({{.*}} : @guaranteed $C):
2126+
// CHECK-NEXT: end_borrow %2
2127+
// CHECK-NEXT: end_borrow %1
2128+
// CHECK: bb2:
2129+
// CHECK-NEXT: end_borrow %2
2130+
// CHECK-NEXT: end_borrow %1
2131+
// CHECK: } // end sil function 'end_borrows_in_liferange_exit_block'
2132+
sil [ossa] @end_borrows_in_liferange_exit_block : $@convention(thin) (@in_guaranteed Optional<C>) -> () {
2133+
bb0(%0 : $*Optional<C>):
2134+
%1 = load [copy] %0
2135+
%2 = move_value [lexical] %1
2136+
switch_enum %2, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
2137+
2138+
bb1(%4 : @owned $C):
2139+
destroy_value %4
2140+
br bb3
2141+
2142+
bb2:
2143+
br bb3
2144+
2145+
bb3:
2146+
%r = tuple ()
2147+
return %r
2148+
}

0 commit comments

Comments
 (0)