Skip to content

Commit 5cedf61

Browse files
committed
Fix MandatoryARCOpts tryJoiningCopyValueLiveRangeWithOperand.
Fix a miscompile in Debug builds at -Onone. This optimization ignores uses of owned values that aren't enclosed in borrow scopes. This is fairly eggregious since project_box instructions are never borrowed, which means that all local variables have this problem. This is a well-known issue that occurs throughout OSSA optimizations. The reason that we don't see the problem often is that the optimizations are hidden behind a pile of ad-hoc pattern matching, so they only kick in for simple cases. This approach to optimization is great at hiding problems for a long time. We're attempting to design away this class of problems in the next release. Until then, it's one miscompile at a time. Fixes rdar://107420448 (Variable mutation in block isn't reflected in outer scope: new behavior in swift 5.9) (cherry picked from commit 26a6264)
1 parent 303fc14 commit 5cedf61

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,12 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
493493
}))
494494
return false;
495495
}
496+
// Check whether the uses considered immediately above are all effectively
497+
// instantaneous uses. Pointer escapes propagate values ways that may not be
498+
// discoverable.
499+
if (hasPointerEscape(operand)) {
500+
return false;
501+
}
496502

497503
// Ok, we now know that we can eliminate this value.
498504
LLVM_DEBUG(llvm::dbgs()

test/SILOptimizer/semantic-arc-opts-lifetime-joining.sil

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
// RUN: %target-sil-opt -module-name Swift -enable-sil-verify-all -semantic-arc-opts -sil-semantic-arc-peepholes-lifetime-joining %s | %FileCheck %s
2-
// REQUIRES: swift_stdlib_asserts
32

43
// NOTE: Some of our tests here depend on borrow elimination /not/ running!
54
// Please do not add it to clean up the IR like we did in
@@ -15,6 +14,13 @@ import Builtin
1514

1615
typealias AnyObject = Builtin.AnyObject
1716

17+
struct Bool {
18+
var value : Builtin.Int1
19+
}
20+
21+
sil @closureCapturesBool : $@convention(thin) (@guaranteed { var Bool }) -> ()
22+
sil @closureArgumentEscapes : $@convention(thin) (@owned @callee_guaranteed () -> ()) -> ()
23+
1824
enum MyNever {}
1925
enum FakeOptional<T> {
2026
case none
@@ -883,3 +889,34 @@ bb3(%result : @owned $FakeOptional<Builtin.NativeObject>):
883889
dealloc_stack %allocStack : $*Builtin.NativeObject
884890
return %result : $FakeOptional<Builtin.NativeObject>
885891
}
892+
893+
// Don't do this optimization:
894+
// Eliminate borrowed copy with useless lifetime:
895+
// %5 = copy_value %0 : ${ var Bool }
896+
//
897+
// CHECK: sil hidden [ossa] @testCapturedSingleDestroyCopy : $@convention(thin) () -> Bool {
898+
// CHECK: load [trivial] %{{.*}} : $*Bool
899+
// CHECK: destroy_value %0 : ${ var Bool }
900+
sil hidden [ossa] @testCapturedSingleDestroyCopy : $@convention(thin) () -> Bool {
901+
bb0:
902+
// var test = false
903+
%0 = alloc_box ${ var Bool }, var, name "test"
904+
%1 = project_box %0 : ${ var Bool }, 0
905+
%2 = integer_literal $Builtin.Int1, 0
906+
%3 = struct $Bool (%2 : $Builtin.Int1)
907+
store %3 to [trivial] %1 : $*Bool
908+
909+
// capture test in an escaping closure
910+
%5 = copy_value %0 : ${ var Bool }
911+
%6 = function_ref @closureCapturesBool : $@convention(thin) (@guaranteed { var Bool }) -> ()
912+
%7 = partial_apply [callee_guaranteed] %6(%5) : $@convention(thin) (@guaranteed { var Bool }) -> ()
913+
%8 = function_ref @closureArgumentEscapes : $@convention(thin) (@owned @callee_guaranteed () -> ()) -> ()
914+
%9 = apply %8(%7) : $@convention(thin) (@owned @callee_guaranteed () -> ()) -> ()
915+
916+
// return test
917+
%10 = begin_access [read] [dynamic] %1 : $*Bool
918+
%11 = load [trivial] %10 : $*Bool
919+
end_access %10 : $*Bool
920+
destroy_value %0 : ${ var Bool }
921+
return %11 : $Bool
922+
}

0 commit comments

Comments
 (0)