Skip to content

Commit 012aa62

Browse files
committed
SIL: when updating borrowed-from instructions, remove the old enclosing values first
When an optimization updates borrowed-from instruction it might be necessary to remove the old enclosing values from the borrowed-from instructions. An optimization might transform the SIL in a way that an existing enclosing value is not valid anymore. Fixes a compiler crash: rdar://142991910
1 parent 0720d4f commit 012aa62

File tree

5 files changed

+49
-30
lines changed

5 files changed

+49
-30
lines changed

SwiftCompilerSources/Sources/Optimizer/Utilities/PhiUpdater.swift

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func updateBorrowedFrom(for phis: some Sequence<Phi>, _ context: some MutatingCo
5050
return
5151
}
5252
if phi.value.ownership == .guaranteed {
53-
createBorrowedFrom(for: phi, context)
53+
createEmptyBorrowedFrom(for: phi, context)
5454
}
5555
}
5656

@@ -113,44 +113,34 @@ private func updateBorrowedFrom(for phi: Phi, _ context: some MutatingContext) -
113113
defer { computedEVs.deinitialize() }
114114
gatherEnclosingValuesFromPredecessors(for: phi, in: &computedEVs, context)
115115

116-
var changed = false
117-
for use in phi.value.uses {
118-
if let bfi = use.forwardingBorrowedFromUser {
119-
changed = addEnclosingValues(computedEVs, to: bfi, context) || changed
120-
}
121-
}
122-
return changed
123-
}
124-
125-
private func createBorrowedFrom(for phi: Phi, _ context: some MutatingContext) {
126-
if !phi.value.uses.contains(where: { $0.forwardingBorrowedFromUser != nil }) {
127-
let builder = Builder(atBeginOf: phi.value.parentBlock, context)
128-
let bfi = builder.createBorrowedFrom(borrowedValue: phi.value, enclosingValues: [])
129-
phi.value.uses.ignoreUsers(ofType: BorrowedFromInst.self).replaceAll(with: bfi, context)
130-
}
131-
}
132-
133-
private func addEnclosingValues(
134-
_ newEVs: some Sequence<Value>,
135-
to borrowedFrom: BorrowedFromInst,
136-
_ context: some MutatingContext) -> Bool
137-
{
116+
let borrowedFrom = phi.borrowedFrom!
138117
var existingEVs = ValueSet(insertContentsOf: borrowedFrom.enclosingValues, context)
139118
defer { existingEVs.deinitialize() }
140119

141-
if newEVs.allSatisfy({ existingEVs.contains($0) }) {
120+
if computedEVs.allSatisfy({ existingEVs.contains($0) }) {
142121
return false
143122
}
144-
145123
var evs = Array<Value>(borrowedFrom.enclosingValues)
146-
evs.append(contentsOf: newEVs.lazy.filter { !existingEVs.contains($0) })
124+
evs.append(contentsOf: computedEVs.lazy.filter { !existingEVs.contains($0) })
147125

148126
let builder = Builder(before: borrowedFrom, context)
149127
let newBfi = builder.createBorrowedFrom(borrowedValue: borrowedFrom.borrowedValue, enclosingValues: evs)
150128
borrowedFrom.replace(with: newBfi, context)
151129
return true
152130
}
153131

132+
private func createEmptyBorrowedFrom(for phi: Phi, _ context: some MutatingContext) {
133+
if let existingBfi = phi.borrowedFrom {
134+
if existingBfi.enclosingValues.isEmpty {
135+
return
136+
}
137+
existingBfi.replace(with: phi.value, context)
138+
}
139+
let builder = Builder(atBeginOf: phi.value.parentBlock, context)
140+
let bfi = builder.createBorrowedFrom(borrowedValue: phi.value, enclosingValues: [])
141+
phi.value.uses.ignoreUsers(ofType: BorrowedFromInst.self).replaceAll(with: bfi, context)
142+
}
143+
154144
/// Replaces a phi with the unique incoming value if all incoming values are the same:
155145
/// ```
156146
/// bb1:

test/SILOptimizer/copy_propagation_borrow.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,7 @@ bb0(%0 : @guaranteed $C):
12091209
}
12101210

12111211
// CHECK-LABEL: sil [ossa] @test_updating_borrowed_from :
1212-
// CHECK: borrowed {{.*}} from {{.*}} %0 : $HasObject
1212+
// CHECK: borrowed {{.*}} from (%0 : $HasObject)
12131213
// CHECK-LABEL: } // end sil function 'test_updating_borrowed_from'
12141214
sil [ossa] @test_updating_borrowed_from : $@convention(thin) (@guaranteed HasObject) -> () {
12151215
bb0(%0 : @guaranteed $HasObject):

test/SILOptimizer/copy_propagation_canonicalize_with_reborrows.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ exit:
3737

3838
// CHECK-LABEL: sil [ossa] @hoist_destroy_over_unrelated_endborrow : {{.*}} {
3939
// CHECK: {{bb[0-9]+}}([[UNRELATED:%[^,]+]] : @reborrow $X, [[VALUE:%[^,]+]] : @owned $X):
40-
// CHECK: destroy_value [[VALUE]]
4140
// CHECK: [[BF:%.*]] = borrowed [[UNRELATED]] : $X from (%0 : $X)
41+
// CHECK: destroy_value [[VALUE]]
4242
// CHECK: end_borrow [[BF]]
4343
// CHECK-LABEL: } // end sil function 'hoist_destroy_over_unrelated_endborrow'
4444
sil [ossa] @hoist_destroy_over_unrelated_endborrow : $@convention(thin) (@guaranteed X) -> () {
@@ -90,8 +90,8 @@ exit:
9090
// CHECK-LABEL: sil [ossa] @hoist_destroy_over_unrelated_reborrow_reborrow_endborrow : {{.*}} {
9191
// CHECK: {{bb[0-9]+}}({{%[^,]+}} : @reborrow $X, {{%[^,]+}} : @owned $X):
9292
// CHECK: {{bb[0-9]+}}([[REBORROW:%[^,]+]] : @reborrow $X, [[VALUE:%[^,]+]] : @owned $X):
93-
// CHECK: destroy_value [[VALUE]]
9493
// CHECK: [[BF:%.*]] = borrowed [[REBORROW]] : $X from (%0 : $X)
94+
// CHECK: destroy_value [[VALUE]]
9595
// CHECK: end_borrow [[BF]]
9696
// CHECK-LABEL: } // end sil function 'hoist_destroy_over_unrelated_reborrow_reborrow_endborrow'
9797
sil [ossa] @hoist_destroy_over_unrelated_reborrow_reborrow_endborrow : $@convention(thin) (@guaranteed X) -> () {

test/SILOptimizer/semantic-arc-opts.sil

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,3 +1612,32 @@ bb4(%4 : @owned $Klass):
16121612
return %t : $()
16131613
}
16141614

1615+
// CHECK-LABEL: sil [ossa] @test_borrowed_from_updating :
1616+
// CHECK: bb3({{.*}}):
1617+
// CHECK-NEXT: borrowed {{.*}} from (%0 : $FakeOptional<Klass>)
1618+
// CHECK-NEXT: borrowed {{.*}} from (%0 : $FakeOptional<Klass>)
1619+
// CHECK-LABEL: } // end sil function 'test_borrowed_from_updating'
1620+
sil [ossa] @test_borrowed_from_updating : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> () {
1621+
bb0(%0 : @guaranteed $FakeOptional<Klass>):
1622+
%1 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
1623+
%2 = begin_borrow %1
1624+
%3 = copy_value %0
1625+
switch_enum %3, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
1626+
1627+
bb1(%5 : @owned $Klass):
1628+
end_borrow %2
1629+
%7 = enum $FakeOptional<Klass>, #FakeOptional.some!enumelt, %5
1630+
%8 = begin_borrow %7
1631+
br bb3(%8, %7)
1632+
1633+
bb2:
1634+
br bb3(%2, %1)
1635+
1636+
bb3(%11 : @reborrow $FakeOptional<Klass>, %12 : @owned $FakeOptional<Klass>):
1637+
%13 = borrowed %11 from (%12)
1638+
end_borrow %13
1639+
destroy_value %12
1640+
%16 = tuple ()
1641+
return %16
1642+
}
1643+

test/SILOptimizer/sil_combine_misc_opts.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ bb0(%0 : $*Builtin.NativeObject):
102102
}
103103

104104
// CHECK-LABEL: sil [ossa] @test_updating_borrowed_from :
105-
// CHECK: borrowed {{.*}} from {{.*}} %0 : $S
105+
// CHECK: borrowed {{.*}} from (%0 : $S)
106106
// CHECK-LABEL: } // end sil function 'test_updating_borrowed_from'
107107
sil [ossa] @test_updating_borrowed_from : $@convention(thin) (@guaranteed S) -> () {
108108
bb0(%0 : @guaranteed $S):

0 commit comments

Comments
 (0)