Skip to content

Commit 04b2d00

Browse files
committed
CSE: disable CSE of lazy property getters of struct
We cannot prove that the whole struct is overwritten between two lazy property getters. We would need AliasAnalysis for this, but currently this is not used in CSE. rdar://problem/67734844
1 parent 8bbffac commit 04b2d00

File tree

3 files changed

+91
-4
lines changed

3 files changed

+91
-4
lines changed

lib/SILOptimizer/Transforms/CSE.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,14 @@ static bool isLazyPropertyGetter(ApplyInst *ai) {
831831
!callee->isLazyPropertyGetter())
832832
return false;
833833

834+
// Only handle classes, but not structs.
835+
// Lazy property getters of structs have an indirect inout self parameter.
836+
// We don't know if the whole struct is overwritten between two getter calls.
837+
// In such a case, the lazy property could be reset to an Optional.none.
838+
// TODO: We could check this case with AliasAnalysis.
839+
if (ai->getArgument(0)->getType().isAddress())
840+
return false;
841+
834842
// Check if the first block has a switch_enum of an Optional.
835843
// We don't handle getters of generic types, which have a switch_enum_addr.
836844
// This will be obsolete with opaque values anyway.

test/SILOptimizer/cse.sil

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,3 +1306,53 @@ bb0(%0 : $@thick SpecialEnum.Type):
13061306
%4 = struct $Bool (%3 : $Builtin.Int1)
13071307
return %4 : $Bool
13081308
}
1309+
1310+
struct StructWithLazyProperty {
1311+
var lazyProperty: Int64 { mutating get set }
1312+
@_hasStorage @_hasInitialValue var lazyPropertyStorage : Int64? { get set }
1313+
}
1314+
1315+
sil private [lazy_getter] [noinline] @lazy_getter : $@convention(method) (@inout StructWithLazyProperty) -> Int64 {
1316+
bb0(%0 : $*StructWithLazyProperty):
1317+
%2 = struct_element_addr %0 : $*StructWithLazyProperty, #StructWithLazyProperty.lazyPropertyStorage
1318+
%3 = load %2 : $*Optional<Int64>
1319+
switch_enum %3 : $Optional<Int64>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
1320+
1321+
bb1(%5 : $Int64):
1322+
br bb3(%5 : $Int64)
1323+
1324+
bb2:
1325+
%9 = integer_literal $Builtin.Int64, 27
1326+
%10 = struct $Int64 (%9 : $Builtin.Int64)
1327+
%12 = enum $Optional<Int64>, #Optional.some!enumelt, %10 : $Int64
1328+
store %12 to %2 : $*Optional<Int64>
1329+
br bb3(%10 : $Int64)
1330+
1331+
bb3(%15 : $Int64):
1332+
return %15 : $Int64
1333+
}
1334+
1335+
sil @take_int : $@convention(thin) (Int64) -> ()
1336+
1337+
// CHECK-LABEL: sil @dont_cse_lazy_property_of_overwritten_struct : $@convention(thin) () -> () {
1338+
// CHECK: [[GETTER:%[0-9]+]] = function_ref @lazy_getter
1339+
// CHECK: apply [[GETTER]]
1340+
// CHECK: apply [[GETTER]]
1341+
// CHECK: } // end sil function 'dont_cse_lazy_property_of_overwritten_struct'
1342+
sil @dont_cse_lazy_property_of_overwritten_struct : $@convention(thin) () -> () {
1343+
bb0:
1344+
%0 = alloc_stack $StructWithLazyProperty
1345+
%4 = enum $Optional<Int64>, #Optional.none!enumelt
1346+
%5 = struct $StructWithLazyProperty (%4 : $Optional<Int64>)
1347+
store %5 to %0 : $*StructWithLazyProperty
1348+
%7 = function_ref @lazy_getter : $@convention(method) (@inout StructWithLazyProperty) -> Int64
1349+
%8 = apply %7(%0) : $@convention(method) (@inout StructWithLazyProperty) -> Int64
1350+
%9 = function_ref @take_int : $@convention(thin) (Int64) -> ()
1351+
%10 = apply %9(%8) : $@convention(thin) (Int64) -> ()
1352+
store %5 to %0 : $*StructWithLazyProperty
1353+
%18 = apply %7(%0) : $@convention(method) (@inout StructWithLazyProperty) -> Int64
1354+
%20 = apply %9(%18) : $@convention(thin) (Int64) -> ()
1355+
dealloc_stack %0 : $*StructWithLazyProperty
1356+
%22 = tuple ()
1357+
return %22 : $()
1358+
}

test/SILOptimizer/lazy_property_getters.swift

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,20 +88,42 @@ func test_no_hoisting(_ c: Myclass, _ b: Bool) -> Int {
8888
return v
8989
}
9090

91+
// This test is disabled, because for structs, it does not work yet.
92+
// CSE is too conservative to handle indirect getter arguments currently.
93+
9194
// CHECK-LABEL: sil {{.*}} @$s4test0A7_structySiAA8MystructVF
95+
// CHECK-DISABLED: [[GETTER:%[0-9]+]] = function_ref @$s4test8MystructV4lvarSivg
96+
// CHECK-DISABLED: [[V1:%[0-9]+]] = apply [[GETTER]]({{.*}})
97+
// CHECK-DISABLED: [[V2OPT:%[0-9]+]] = load
98+
// CHECK-DISABLED: [[V2:%[0-9]+]] = unchecked_enum_data [[V2OPT]]
99+
// CHECK-DISABLED: [[V1VAL:%[0-9]+]] = struct_extract [[V1]]
100+
// CHECK-DISABLED: [[V2VAL:%[0-9]+]] = struct_extract [[V2]]
101+
// CHECK-DISABLED: builtin "sadd{{.*}}"([[V1VAL]] {{.*}}, [[V2VAL]]
102+
// CHECK-DISABLED: } // end sil function '$s4test0A7_structySiAA8MystructVF'
103+
@inline(never)
104+
func test_struct(_ s: Mystruct) -> Int {
105+
var sm = s
106+
g = 42
107+
let v1 = sm.lvar
108+
let v2 = sm.lvar
109+
return v1 &+ v2
110+
}
111+
112+
// CHECK-LABEL: sil {{.*}} @$s4test0A19_overwritten_structySiAA8MystructVF
92113
// CHECK: [[GETTER:%[0-9]+]] = function_ref @$s4test8MystructV4lvarSivg
93114
// CHECK: [[V1:%[0-9]+]] = apply [[GETTER]]({{.*}})
94-
// CHECK: [[V2OPT:%[0-9]+]] = load
95-
// CHECK: [[V2:%[0-9]+]] = unchecked_enum_data [[V2OPT]]
115+
// CHECK: [[V2:%[0-9]+]] = apply [[GETTER]]({{.*}})
96116
// CHECK: [[V1VAL:%[0-9]+]] = struct_extract [[V1]]
97117
// CHECK: [[V2VAL:%[0-9]+]] = struct_extract [[V2]]
98118
// CHECK: builtin "sadd{{.*}}"([[V1VAL]] {{.*}}, [[V2VAL]]
99-
// CHECK: } // end sil function '$s4test0A7_structySiAA8MystructVF'
119+
// CHECK: } // end sil function '$s4test0A19_overwritten_structySiAA8MystructVF'
100120
@inline(never)
101-
func test_struct(_ s: Mystruct) -> Int {
121+
func test_overwritten_struct(_ s: Mystruct) -> Int {
102122
var sm = s
103123
g = 42
104124
let v1 = sm.lvar
125+
sm = s
126+
g = 43
105127
let v2 = sm.lvar
106128
return v1 &+ v2
107129
}
@@ -133,6 +155,13 @@ func calltests() {
133155
// CHECK-OUTPUT-NEXT: lvar init
134156
// CHECK-OUTPUT-NEXT: 84
135157
print(test_struct(Mystruct()))
158+
159+
// CHECK-OUTPUT-LABEL: test_overwritten_struct
160+
print("test_overwritten_struct")
161+
// CHECK-OUTPUT-NEXT: lvar init
162+
// CHECK-OUTPUT-NEXT: lvar init
163+
// CHECK-OUTPUT-NEXT: 85
164+
print(test_overwritten_struct(Mystruct()))
136165
}
137166

138167
calltests()

0 commit comments

Comments
 (0)