Skip to content

Commit b8b2fc4

Browse files
authored
[5.5] CSE: While optimizing lazy property getters, don't inline non-ossa to ossa function (#38231)
Fixes rdar://79781904 Cherry-pick of #38184 • Explanation: Fixes a bug in optimizing lazy property getters in CSE. Lazy property getters are CSE'ed by inlining their function body. There was no check to skip inlining a non-ossa function into an ossa function which is illegal. • Scope of Issue: Can lead to crashes or miscompilations • Risk: Low. The fix is low risk because it is just avoiding the illegal CSE of lazy property getters. • Reviewed By: Erik Eckstein • Testing: Added unit tests
1 parent 81cd8ce commit b8b2fc4

File tree

3 files changed

+82
-0
lines changed

3 files changed

+82
-0
lines changed

lib/SILOptimizer/Transforms/CSE.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,10 @@ static bool isLazyPropertyGetter(ApplyInst *ai) {
915915
!callee->isLazyPropertyGetter())
916916
return false;
917917

918+
// We cannot inline a non-ossa function into an ossa function
919+
if (ai->getFunction()->hasOwnership() && !callee->hasOwnership())
920+
return false;
921+
918922
// Only handle classes, but not structs.
919923
// Lazy property getters of structs have an indirect inout self parameter.
920924
// We don't know if the whole struct is overwritten between two getter calls.

lib/SILOptimizer/Utils/SILInliner.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,9 @@ SILInliner::inlineFullApply(FullApplySite apply,
348348
SILInliner::InlineKind inlineKind,
349349
SILOptFunctionBuilder &funcBuilder) {
350350
assert(apply.canOptimize());
351+
assert(!apply.getFunction()->hasOwnership() ||
352+
apply.getReferencedFunctionOrNull()->hasOwnership());
353+
351354
SmallVector<SILValue, 8> appliedArgs;
352355
for (const auto arg : apply.getArguments())
353356
appliedArgs.push_back(arg);

test/SILOptimizer/cse_ossa.sil

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,3 +1223,78 @@ bb0(%0 : $*Builtin.Int64, %1 : @guaranteed $Builtin.NativeObject):
12231223
destroy_value %copy : $Builtin.NativeObject
12241224
return %6 : $(Builtin.Int64, Builtin.Int64)
12251225
}
1226+
1227+
final class LazyKlass {
1228+
final var lazyProperty: Int64 { get set }
1229+
@_hasStorage @_hasInitialValue final var lazyPropertyStorage : Int64? { get set }
1230+
init()
1231+
}
1232+
1233+
sil private [lazy_getter] [noinline] @lazy_class_getter_nonossa : $@convention(method) (@guaranteed LazyKlass) -> Int64 {
1234+
bb0(%0 : $LazyKlass):
1235+
%2 = ref_element_addr %0 : $LazyKlass, #LazyKlass.lazyPropertyStorage
1236+
%3 = load %2 : $*Optional<Int64>
1237+
switch_enum %3 : $Optional<Int64>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
1238+
1239+
bb1(%5 : $Int64):
1240+
br bb3(%5 : $Int64)
1241+
1242+
bb2:
1243+
%9 = integer_literal $Builtin.Int64, 27
1244+
%10 = struct $Int64 (%9 : $Builtin.Int64)
1245+
%12 = enum $Optional<Int64>, #Optional.some!enumelt, %10 : $Int64
1246+
store %12 to %2 : $*Optional<Int64>
1247+
br bb3(%10 : $Int64)
1248+
1249+
bb3(%15 : $Int64):
1250+
return %15 : $Int64
1251+
}
1252+
1253+
sil private [lazy_getter] [noinline] [ossa] @lazy_class_getter_ossa : $@convention(method) (@guaranteed LazyKlass) -> Int64 {
1254+
bb0(%0 : @guaranteed $LazyKlass):
1255+
%2 = ref_element_addr %0 : $LazyKlass, #LazyKlass.lazyPropertyStorage
1256+
%3 = load [trivial] %2 : $*Optional<Int64>
1257+
switch_enum %3 : $Optional<Int64>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
1258+
1259+
bb1(%5 : $Int64):
1260+
br bb3(%5 : $Int64)
1261+
1262+
bb2:
1263+
%9 = integer_literal $Builtin.Int64, 27
1264+
%10 = struct $Int64 (%9 : $Builtin.Int64)
1265+
%12 = enum $Optional<Int64>, #Optional.some!enumelt, %10 : $Int64
1266+
store %12 to [trivial] %2 : $*Optional<Int64>
1267+
br bb3(%10 : $Int64)
1268+
1269+
bb3(%15 : $Int64):
1270+
return %15 : $Int64
1271+
}
1272+
1273+
// CHECK-LABEL: sil [ossa] @dont_cse_nonossa_getter :
1274+
// CHECK: [[GETTER:%[0-9]+]] = function_ref @lazy_class_getter_nonossa
1275+
// CHECK: apply [[GETTER]]
1276+
// CHECK: apply [[GETTER]]
1277+
// CHECK: } // end sil function 'dont_cse_nonossa_getter'
1278+
sil [ossa] @dont_cse_nonossa_getter : $@convention(thin) (@guaranteed LazyKlass) -> (Int64, Int64) {
1279+
bb0(%0 : @guaranteed $LazyKlass):
1280+
%getter = function_ref @lazy_class_getter_nonossa : $@convention(method) (@guaranteed LazyKlass) -> Int64
1281+
%call1 = apply %getter(%0) : $@convention(method) (@guaranteed LazyKlass) -> Int64
1282+
%call2 = apply %getter(%0) : $@convention(method) (@guaranteed LazyKlass) -> Int64
1283+
%res = tuple (%call1 : $Int64, %call2 : $Int64)
1284+
return %res : $(Int64, Int64)
1285+
}
1286+
1287+
// CHECK-LABEL: sil @cse_ossa_getter :
1288+
// CHECK: [[GETTER:%[0-9]+]] = function_ref @lazy_class_getter_ossa
1289+
// CHECK: apply [[GETTER]]
1290+
// CHECK-NOT: apply [[GETTER]]
1291+
// CHECK: } // end sil function 'cse_ossa_getter'
1292+
sil @cse_ossa_getter : $@convention(thin) (@guaranteed LazyKlass) -> (Int64, Int64) {
1293+
bb0(%0 : $LazyKlass):
1294+
%1 = function_ref @lazy_class_getter_ossa : $@convention(method) (@guaranteed LazyKlass) -> Int64
1295+
%2 = apply %1(%0) : $@convention(method) (@guaranteed LazyKlass) -> Int64
1296+
%3 = apply %1(%0) : $@convention(method) (@guaranteed LazyKlass) -> Int64
1297+
%4 = tuple (%2 : $Int64, %3 : $Int64)
1298+
return %4 : $(Int64, Int64)
1299+
}
1300+

0 commit comments

Comments
 (0)