Skip to content

Commit 6c2d3eb

Browse files
committed
Bailout of CopyValueOpts when there is a pointer escape
Copy propagation does these optimizations more generally. It should be able to replace this optimization. Fixing it and not just deleting CopyValueOpts because it happens to be called in the mandatory pipeline.
1 parent cc14548 commit 6c2d3eb

File tree

3 files changed

+120
-6
lines changed

3 files changed

+120
-6
lines changed

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,11 @@ static bool isUseBetweenInstAndBlockEnd(
388388
static bool tryJoinIfDestroyConsumingUseInSameBlock(
389389
SemanticARCOptVisitor &ctx, CopyValueInst *cvi, DestroyValueInst *dvi,
390390
SILValue operand, Operand *singleCVIConsumingUse) {
391+
// Pointer escapes propagate values ways that may not be discoverable.
392+
// If \p cvi or \p operand has escaped, then do not optimize.
393+
if (findPointerEscape(cvi) || findPointerEscape(operand)) {
394+
return false;
395+
}
391396
// First see if our destroy_value is in between singleCVIConsumingUse and the
392397
// end of block. If this is not true, then we know the destroy_value must be
393398
// /before/ our singleCVIConsumingUse meaning that by joining the lifetimes,
@@ -517,12 +522,6 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
517522
}))
518523
return false;
519524
}
520-
// Check whether the uses considered immediately above are all effectively
521-
// instantaneous uses. Pointer escapes propagate values ways that may not be
522-
// discoverable.
523-
if (findPointerEscape(operand)) {
524-
return false;
525-
}
526525

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

test/SILOptimizer/definite-init-convert-to-escape.swift

+2
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,12 @@ public func returnOptionalEscape() -> (() ->())?
9292
// NOPEEPHOLE: switch_enum [[V1]] : $Optional<{{.*}}>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[NONE_BB:bb[0-9]+]]
9393
//
9494
// NOPEEPHOLE: [[SOME_BB]]([[V2:%.*]]: $@callee_guaranteed () -> ()):
95+
// NOPEEPHOLE-NEXT: strong_retain [[V2]]
9596
// NOPEEPHOLE-NEXT: [[CVT:%.*]] = convert_escape_to_noescape [[V2]]
9697
// NOPEEPHOLE-NEXT: [[SOME:%.*]] = enum $Optional<{{.*}}>, #Optional.some!enumelt, [[V2]]
9798
// NOPEEPHOLE-NEXT: [[MDI:%.*]] = mark_dependence [[CVT]] : $@noescape @callee_guaranteed () -> () on [[SOME]] : $Optional<@callee_guaranteed () -> ()>
9899
// NOPEEPHOLE-NEXT: [[NOESCAPE_SOME:%.*]] = enum $Optional<{{.*}}>, #Optional.some!enumelt, [[MDI]]
100+
// NOPEEPHOLE-NEXT: strong_release [[V2]]
99101
// NOPEEPHOLE-NEXT: br bb2([[NOESCAPE_SOME]] : $Optional<{{.*}}>, [[SOME]] : $Optional<{{.*}}>, [[SOME]] : $Optional<{{.*}}>)
100102
//
101103
// NOPEEPHOLE: bb2([[NOESCAPE_SOME:%.*]] : $Optional<{{.*}}>, [[SOME1:%.*]] : $Optional<{{.*}}>, [[SOME:%.*]] : $Optional<{{.*}}>):

test/SILOptimizer/rdar146142041.sil

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -semantic-arc-opts | %FileCheck %s
2+
3+
import Swift
4+
5+
class Klass {
6+
init()
7+
}
8+
9+
struct NonTrivial {
10+
@_hasStorage let k: Klass { get }
11+
init(k: Klass)
12+
}
13+
14+
sil @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
15+
16+
// CHECK-LABEL: sil [ossa] @test1 :
17+
// CHECK: [[PA:%.*]] = partial_apply
18+
// CHECK: [[COPY:%.*]] = copy_value [[PA]]
19+
// CHECK-LABEL: } // end sil function 'test1'
20+
sil [ossa] @test1 : $@convention(thin) () -> () {
21+
bb0:
22+
cond_br undef, bb2, bb1
23+
24+
bb1:
25+
%1 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.none!enumelt
26+
br bb7(%1)
27+
28+
bb2:
29+
br bb3
30+
31+
bb3:
32+
%4 = alloc_box ${ var Optional<NonTrivial> }
33+
%5 = begin_borrow [lexical] %4
34+
%6 = project_box %5, 0
35+
inject_enum_addr %6, #Optional.none!enumelt
36+
cond_br undef, bb4, bb5
37+
38+
bb4:
39+
unreachable
40+
41+
bb5:
42+
br bb6
43+
44+
bb6:
45+
%11 = function_ref @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
46+
%12 = partial_apply [callee_guaranteed] %11(%6) : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
47+
%13 = copy_value %12
48+
%14 = convert_escape_to_noescape %13 to $@noescape @callee_guaranteed () -> ()
49+
%15 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt, %13
50+
end_borrow %5
51+
destroy_value %12
52+
destroy_value %4
53+
destroy_value %14
54+
br bb7(%15)
55+
56+
bb7(%21 : @owned $Optional<@callee_guaranteed () -> ()>):
57+
destroy_value %21
58+
br bb8
59+
60+
bb8:
61+
%24 = tuple ()
62+
return %24
63+
}
64+
65+
// CHECK-LABEL: sil [ossa] @test2 :
66+
// CHECK: [[PA:%.*]] = partial_apply
67+
// CHECK: [[COPY:%.*]] = copy_value [[PA]]
68+
// CHECK-LABEL: } // end sil function 'test2'
69+
sil [ossa] @test2 : $@convention(thin) () -> () {
70+
bb0:
71+
cond_br undef, bb2, bb1
72+
73+
bb1:
74+
%1 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.none!enumelt
75+
br bb7(%1)
76+
77+
bb2:
78+
br bb3
79+
80+
bb3:
81+
%4 = alloc_box ${ var Optional<NonTrivial> }
82+
%5 = begin_borrow [lexical] %4
83+
%6 = project_box %5, 0
84+
inject_enum_addr %6, #Optional.none!enumelt
85+
cond_br undef, bb4, bb5
86+
87+
bb4:
88+
unreachable
89+
90+
bb5:
91+
br bb6
92+
93+
bb6:
94+
%11 = function_ref @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
95+
%12 = partial_apply [callee_guaranteed] %11(%6) : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
96+
%13 = copy_value %12
97+
destroy_value %12
98+
%14 = convert_escape_to_noescape %13 to $@noescape @callee_guaranteed () -> ()
99+
%15 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt, %13
100+
end_borrow %5
101+
destroy_value %4
102+
destroy_value %14
103+
br bb7(%15)
104+
105+
bb7(%21 : @owned $Optional<@callee_guaranteed () -> ()>):
106+
destroy_value %21
107+
br bb8
108+
109+
bb8:
110+
%24 = tuple ()
111+
return %24
112+
}
113+

0 commit comments

Comments
 (0)