Skip to content

Commit 2e92644

Browse files
eecksteineeckstein
authored
Merge pull request #78754 from eeckstein/fix-alias-analysis
AliasAnalysis: consider memory effects of a consume/destroy of a class on it's let-fields
2 parents 11fbd94 + 97db5d8 commit 2e92644

File tree

5 files changed

+133
-30
lines changed

5 files changed

+133
-30
lines changed

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -378,15 +378,6 @@ struct AliasAnalysis {
378378
// The address has unknown escapes. So we have to take the global effects of the called function(s).
379379
memoryEffects = calleeAnalysis.getSideEffects(ofApply: apply).memory
380380
}
381-
// Do some magic for `let` variables. Function calls cannot modify let variables.
382-
// The only exception is that the let variable is directly passed to an indirect out of the apply.
383-
// TODO: make this a more formal and verified approach.
384-
if memoryEffects.write {
385-
let accessBase = memLoc.address.accessBase
386-
if accessBase.isLet && !accessBase.isIndirectResult(of: apply) {
387-
return SideEffects.Memory(read: memoryEffects.read, write: false)
388-
}
389-
}
390381
return memoryEffects
391382
}
392383

@@ -440,11 +431,7 @@ struct AliasAnalysis {
440431
initialWalkingDirection: memLoc.walkingDirection,
441432
complexityBudget: getComplexityBudget(for: inst.parentFunction), context)
442433
{
443-
var effects = inst.memoryEffects
444-
if memLoc.isLetValue {
445-
effects.write = false
446-
}
447-
return effects
434+
return inst.memoryEffects
448435
}
449436
return .noEffects
450437
}
@@ -628,6 +615,12 @@ private enum ImmutableScope {
628615
return nil
629616
}
630617
object = tailAddr.instance
618+
case .global(let global):
619+
if global.isLet && !basedAddress.parentFunction.canInitializeGlobal {
620+
self = .wholeFunction
621+
return
622+
}
623+
return nil
631624
default:
632625
return nil
633626
}
@@ -907,6 +900,14 @@ private extension Type {
907900
}
908901
}
909902

903+
private extension Function {
904+
var canInitializeGlobal: Bool {
905+
return isGlobalInitOnceFunction ||
906+
// In non -parse-as-library mode globals are initialized in the `main` function.
907+
name == "main"
908+
}
909+
}
910+
910911
//===--------------------------------------------------------------------===//
911912
// Bridging
912913
//===--------------------------------------------------------------------===//

‎SwiftCompilerSources/Sources/Optimizer/TestPasses/MemBehaviorDumper.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ private extension Instruction {
6565
is EndCOWMutationInst,
6666
is CopyValueInst,
6767
is DestroyValueInst,
68+
is StrongReleaseInst,
6869
is IsUniqueInst,
6970
is EndBorrowInst,
7071
is LoadInst,

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ sil [ossa] @dont_copy_let_properties_with_guaranteed_base : $@convention(thin) (
199199
bb0(%x : @guaranteed $ClassLet):
200200
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
201201

202-
%p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
202+
%p = ref_element_addr [immutable] %x : $ClassLet, #ClassLet.aLet
203203
%v = load [copy] %p : $*Klass
204204
%b = begin_borrow %v : $Klass
205205
apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
@@ -223,7 +223,7 @@ sil [ossa] @dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses :
223223
bb0(%x : @guaranteed $ClassLet):
224224
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
225225

226-
%p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
226+
%p = ref_element_addr [immutable] %x : $ClassLet, #ClassLet.aLet
227227
%v = load [copy] %p : $*Klass
228228
%c = unchecked_ref_cast %v : $Klass to $Klass
229229
%b = begin_borrow %c : $Klass
@@ -247,7 +247,7 @@ bb0(%x : @guaranteed $SubclassLet):
247247
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
248248

249249
%u = upcast %x : $SubclassLet to $ClassLet
250-
%p = ref_element_addr %u : $ClassLet, #ClassLet.aLet
250+
%p = ref_element_addr [immutable] %u : $ClassLet, #ClassLet.aLet
251251
%v = load [copy] %p : $*Klass
252252
%b = begin_borrow %v : $Klass
253253
apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
@@ -293,7 +293,7 @@ sil [ossa] @dont_copy_let_properties_with_guaranteed_base_structural : $@convent
293293
bb0(%x : @guaranteed $ClassLet):
294294
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
295295

296-
%p = ref_element_addr %x : $ClassLet, #ClassLet.aLetTuple
296+
%p = ref_element_addr [immutable] %x : $ClassLet, #ClassLet.aLetTuple
297297
%q = tuple_element_addr %p : $*(Klass, Klass), 1
298298
%v = load [copy] %q : $*Klass
299299
%b = begin_borrow %v : $Klass
@@ -369,7 +369,7 @@ bb0(%x : @owned $ClassLet):
369369
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
370370

371371
%a = begin_borrow %x : $ClassLet
372-
%p = ref_element_addr %a : $ClassLet, #ClassLet.aLet
372+
%p = ref_element_addr [immutable] %a : $ClassLet, #ClassLet.aLet
373373
%v = load [copy] %p : $*Klass
374374
apply %f(%v) : $@convention(thin) (@guaranteed Klass) -> ()
375375
destroy_value %v : $Klass
@@ -409,7 +409,7 @@ bb0(%x : @owned $ClassLet):
409409
%f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
410410

411411
%a = begin_borrow %x : $ClassLet
412-
%p = ref_element_addr %a : $ClassLet, #ClassLet.aLet
412+
%p = ref_element_addr [immutable] %a : $ClassLet, #ClassLet.aLet
413413
%v = load [copy] %p : $*Klass
414414
%v_cast = unchecked_ref_cast %v : $Klass to $Builtin.NativeObject
415415
apply %f(%v_cast) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
@@ -1145,7 +1145,7 @@ bb0(%0 : @guaranteed $FakeOptional<ClassLet>):
11451145
switch_enum %0 : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
11461146

11471147
bb1(%1 : @guaranteed $ClassLet):
1148-
%2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
1148+
%2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
11491149
%3 = load [copy] %2 : $*Klass
11501150
apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
11511151
destroy_value %3 : $Klass
@@ -1171,7 +1171,7 @@ bb0(%0 : @owned $FakeOptional<ClassLet>):
11711171
switch_enum %0a : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
11721172

11731173
bb1(%1 : @guaranteed $ClassLet):
1174-
%2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
1174+
%2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
11751175
%3 = load [copy] %2 : $*Klass
11761176
apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
11771177
destroy_value %3 : $Klass
@@ -1201,7 +1201,7 @@ bb0(%0 : $*FakeOptional<ClassLet>):
12011201
switch_enum %0a : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
12021202

12031203
bb1(%1 : @guaranteed $ClassLet):
1204-
%2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
1204+
%2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
12051205
%3 = load [copy] %2 : $*Klass
12061206
apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
12071207
destroy_value %3 : $Klass
@@ -1240,7 +1240,7 @@ bb0c(%0c : @guaranteed $FakeOptional<ClassLet>):
12401240
switch_enum %0d : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
12411241

12421242
bb1(%1 : @guaranteed $ClassLet):
1243-
%2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
1243+
%2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
12441244
%3 = load [copy] %2 : $*Klass
12451245
apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
12461246
destroy_value %3 : $Klass
@@ -1276,7 +1276,7 @@ bb1:
12761276
bb2:
12771277
%0b = begin_borrow %0 : $FakeOptional<ClassLet>
12781278
%0b2 = unchecked_enum_data %0b : $FakeOptional<ClassLet>, #FakeOptional.some!enumelt
1279-
%2 = ref_element_addr %0b2 : $ClassLet, #ClassLet.aLet
1279+
%2 = ref_element_addr [immutable] %0b2 : $ClassLet, #ClassLet.aLet
12801280
%3 = load [copy] %2 : $*Klass
12811281
apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
12821282
destroy_value %3 : $Klass
@@ -1520,12 +1520,9 @@ bbEnd:
15201520
return %9999 : $()
15211521
}
15221522

1523-
// Just make sure that we do not crash on this code and convert the 2nd load
1524-
// [copy] to a load_borrow.
1523+
// Just make sure that we do not crash on this code
15251524
//
15261525
// CHECK-LABEL: sil [ossa] @improper_dead_end_block_crasher_test : $@convention(thin) (Builtin.RawPointer) -> () {
1527-
// CHECK: load_borrow
1528-
// CHECK: load_borrow
15291526
// CHECK: } // end sil function 'improper_dead_end_block_crasher_test'
15301527
sil [ossa] @improper_dead_end_block_crasher_test : $@convention(thin) (Builtin.RawPointer) -> () {
15311528
bb0(%0 : $Builtin.RawPointer):

‎test/SILOptimizer/mem-behavior.sil‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ class C {
1919
@_hasStorage @_hasInitialValue final var prop: Builtin.Int32 { get }
2020
}
2121

22+
class CL {
23+
@_hasStorage let x: String
24+
}
25+
26+
2227
class Parent {
2328
@_hasStorage var child: C { get set }
2429
}
@@ -34,6 +39,7 @@ sil @nouser_func : $@convention(thin) () -> ()
3439
sil @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
3540
sil @init_C : $@convention(thin) () -> @out C
3641
sil @read_C : $@convention(thin) (@in_guaranteed C) -> ()
42+
sil @consume : $@convention(thin) (@owned CL) -> ()
3743

3844
sil @store_to_int : $@convention(thin) (Int32, @inout Int32) -> () {
3945
[%1: write v**]
@@ -857,7 +863,22 @@ bb0:
857863
// CHECK-NEXT: %2 = apply %1(%0) : $@convention(thin) () -> @out C
858864
// CHECK-NEXT: %0 = global_addr @globalC : $*C
859865
// CHECK-NEXT: r=1,w=1
860-
sil hidden @testInitGlobalLet : $@convention(thin) () -> () {
866+
sil hidden [global_init_once_fn] @testInitGlobalLet : $@convention(thin) () -> () {
867+
bb0:
868+
%0 = global_addr @globalC : $*C
869+
%1 = function_ref @init_C : $@convention(thin) () -> @out C
870+
%2 = apply %1(%0) : $@convention(thin) () -> @out C
871+
%3 = load %0 : $*C
872+
%8 = tuple ()
873+
return %8 : $()
874+
}
875+
876+
// CHECK-LABEL: @main
877+
// CHECK: PAIR #0.
878+
// CHECK-NEXT: %2 = apply %1(%0) : $@convention(thin) () -> @out C
879+
// CHECK-NEXT: %0 = global_addr @globalC : $*C
880+
// CHECK-NEXT: r=1,w=1
881+
sil [global_init_once_fn] @main : $@convention(thin) () -> () {
861882
bb0:
862883
%0 = global_addr @globalC : $*C
863884
%1 = function_ref @init_C : $@convention(thin) () -> @out C
@@ -1788,3 +1809,45 @@ bb0(%0 : $*X):
17881809
return %2 : $C
17891810
}
17901811

1812+
// CHECK-LABEL: @test_release_of_class_with_let
1813+
// CHECK: PAIR #0.
1814+
// CHECK-NEXT: strong_release %0 : $CL
1815+
// CHECK-NEXT: %1 = ref_element_addr [immutable] %0 : $CL, #CL.x
1816+
// CHECK-NEXT: r=1,w=1
1817+
sil @test_release_of_class_with_let : $@convention(thin) (@owned CL) -> () {
1818+
bb0(%0 : $CL):
1819+
%1 = ref_element_addr [immutable] %0, #CL.x
1820+
strong_release %0
1821+
%3 = tuple ()
1822+
return %3
1823+
}
1824+
1825+
// CHECK-LABEL: @test_consume_of_class_with_let
1826+
// CHECK: PAIR #0.
1827+
// CHECK-NEXT: %3 = apply %2(%0) : $@convention(thin) (@owned CL) -> ()
1828+
// CHECK-NEXT: %1 = ref_element_addr [immutable] %0 : $CL, #CL.x
1829+
// CHECK-NEXT: r=1,w=1
1830+
sil @test_consume_of_class_with_let : $@convention(thin) (@owned CL) -> () {
1831+
bb0(%0 : $CL):
1832+
%1 = ref_element_addr [immutable] %0, #CL.x
1833+
%4 = function_ref @consume : $@convention(thin) (@owned CL) -> ()
1834+
apply %4(%0) : $@convention(thin) (@owned CL) -> ()
1835+
%3 = tuple ()
1836+
return %3
1837+
}
1838+
1839+
// CHECK-LABEL: @test_ossa_consume_of_class_with_let
1840+
// CHECK: PAIR #1.
1841+
// CHECK-NEXT: %5 = apply %4(%0) : $@convention(thin) (@owned CL) -> ()
1842+
// CHECK-NEXT: %2 = ref_element_addr [immutable] %1 : $CL, #CL.x
1843+
// CHECK-NEXT: r=1,w=1
1844+
sil [ossa] @test_ossa_consume_of_class_with_let : $@convention(thin) (@owned CL) -> () {
1845+
bb0(%0 : @owned $CL):
1846+
%1 = begin_borrow %0
1847+
%2 = ref_element_addr [immutable] %1, #CL.x
1848+
end_borrow %1
1849+
%4 = function_ref @consume : $@convention(thin) (@owned CL) -> ()
1850+
apply %4(%0) : $@convention(thin) (@owned CL) -> ()
1851+
%3 = tuple ()
1852+
return %3
1853+
}

‎test/SILOptimizer/temp_rvalue_opt.sil‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,15 @@ struct Str {
2525
var _value: Builtin.Int64
2626
}
2727

28+
class C {
29+
@_hasStorage let x: String
30+
}
31+
32+
2833
sil @unknown : $@convention(thin) () -> ()
2934
sil @load_string : $@convention(thin) (@in_guaranteed String) -> String
3035
sil @guaranteed_user : $@convention(thin) (@guaranteed Klass) -> ()
36+
sil @consume : $@convention(thin) (@owned C) -> ()
3137

3238
sil @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> () {
3339
bb0(%0 : $*Klass):
@@ -761,3 +767,38 @@ bb0(%0 : $Klass, %1 : $Klass):
761767
dealloc_stack %3 : $*String
762768
return %323 : $String
763769
}
770+
771+
// CHECK-LABEL: sil @dont_remove_copy_from_released_object
772+
// CHECK: copy_addr
773+
// CHECK-NEXT: strong_release %0
774+
// CHECK: } // end sil function 'dont_remove_copy_from_released_object'
775+
sil @dont_remove_copy_from_released_object : $@convention(thin) (@owned C) -> @owned String {
776+
bb0(%0 : $C):
777+
%1 = ref_element_addr [immutable] %0, #C.x
778+
%2 = alloc_stack $String
779+
copy_addr %1 to [init] %2
780+
strong_release %0
781+
%5 = load %2
782+
retain_value %5
783+
destroy_addr %2
784+
dealloc_stack %2
785+
return %5
786+
}
787+
788+
// CHECK-LABEL: sil @dont_remove_copy_from_consumed_object
789+
// CHECK: copy_addr
790+
// CHECK: apply
791+
// CHECK: } // end sil function 'dont_remove_copy_from_consumed_object'
792+
sil @dont_remove_copy_from_consumed_object : $@convention(thin) (@owned C) -> @owned String {
793+
bb0(%0 : $C):
794+
%1 = ref_element_addr [immutable] %0, #C.x
795+
%2 = alloc_stack $String
796+
copy_addr %1 to [init] %2
797+
%4 = function_ref @consume : $@convention(thin) (@owned C) -> ()
798+
apply %4(%0) : $@convention(thin) (@owned C) -> ()
799+
%5 = load %2
800+
retain_value %5
801+
destroy_addr %2
802+
dealloc_stack %2
803+
return %5
804+
}

0 commit comments

Comments
 (0)