Skip to content

Commit fb943cc

Browse files
committed
[OSLogOptimization] Make emitCodeForSymbolicValue function generate
code for closures iff the closure is not created by the caller function where the code is generated. Otherwise, when the closure is created by the caller, just reuse it after copying and extending its lifetime. Before this change new closures were created as long as all captures of the closures were symbolic constants. This patch updates it so that even if all captures are symbolic constants no code is generated for closures that are already available in the caller. This avoids doing needless work and also fixes the following bug. <rdar://problem/61465764>
1 parent 25d531c commit fb943cc

File tree

2 files changed

+41
-10
lines changed

2 files changed

+41
-10
lines changed

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -728,17 +728,22 @@ static SILValue emitCodeForSymbolicValue(SymbolicValue symVal,
728728
SILModule &module = builder.getModule();
729729
SymbolicClosure *closure = symVal.getClosure();
730730
SILValue resultVal;
731-
if (!closure->hasOnlyConstantCaptures()) {
732-
// If the closure captures a value that is not a constant, it should only
733-
// come from the caller of the log call. Therefore, assert this and reuse
734-
// the closure value.
735-
SingleValueInstruction *originalClosureInst = closure->getClosureInst();
736-
SILFunction &fun = builder.getFunction();
737-
assert(originalClosureInst->getFunction() == &fun &&
738-
"closure with non-constant captures not defined in this function");
739-
// Copy the closure, since the returned value must be owned.
731+
732+
// If the closure was created in the context of this function where the code
733+
// is generated, reuse the original closure value (after extending its
734+
// lifetime by copying).
735+
SingleValueInstruction *originalClosureInst = closure->getClosureInst();
736+
SILFunction &fun = builder.getFunction();
737+
if (originalClosureInst->getFunction() == &fun) {
738+
// Copy the closure, since the returned value must be owned and the
739+
// closure's lifetime must be extended until this point.
740740
resultVal = makeOwnedCopyOfSILValue(originalClosureInst, fun);
741741
} else {
742+
// If the closure captures a value that is not a constant, it should only
743+
// come from the caller of the log call. It should be handled by the then
744+
// case and we should never reach here. Assert this.
745+
assert(closure->hasOnlyConstantCaptures() &&
746+
"closure with non-constant captures not defined in this function");
742747
SubstitutionMap callSubstMap = closure->getCallSubstitutionMap();
743748
ArrayRef<SymbolicClosureArgument> captures = closure->getCaptures();
744749
// Recursively emit code for all captured values which must be mapped to a

test/SILOptimizer/OSLogMandatoryOptTest.swift

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,8 @@ protocol Proto {
597597
var property: String { get set }
598598
}
599599

600-
// Test capturing of address-only types in autoclosures.
600+
// Test capturing of address-only types in autoclosures. The following tests check that
601+
// there are no crashes in these cases.
601602

602603
// CHECK-LABEL: @${{.*}}testInterpolationOfExistentials1pyAA5Proto_p_tF
603604
func testInterpolationOfExistentials(p: Proto) {
@@ -609,3 +610,28 @@ func testInterpolationOfGenerics<T : Proto>(p: T) {
609610
_osLogTestHelper("A generic argument's property \(p.property)")
610611
}
611612

613+
class TestClassSelfTypeCapture {
614+
// CHECK-LABEL: @${{.*}}TestClassSelfTypeCaptureC04testdeF0yyF
615+
func testSelfTypeCapture() {
616+
_osLogTestHelper("Self type of a class \(String(describing: Self.self))")
617+
}
618+
}
619+
620+
struct TestStructSelfTypeCapture {
621+
// CHECK-LABEL: @${{.*}}TestStructSelfTypeCaptureV04testdeF0yyF
622+
func testSelfTypeCapture() {
623+
_osLogTestHelper("Self type of a struct \(String(describing: Self.self))")
624+
}
625+
}
626+
627+
protocol TestProtocolSelfTypeCapture {
628+
func testSelfTypeCapture()
629+
}
630+
631+
extension TestProtocolSelfTypeCapture {
632+
// CHECK-LABEL: @${{.*}}TestProtocolSelfTypeCapturePAAE04testdeF0yyF
633+
func testSelfTypeCapture() {
634+
_osLogTestHelper("Self type of a protocol \(String(describing: Self.self))")
635+
}
636+
}
637+

0 commit comments

Comments
 (0)