Skip to content

Commit 07fb13f

Browse files
slavapestovslavapestov
committed
SILOptimizer: Fix speculative devirtualization to correctly use casted value
We used to emit this: checked_cast_br [exact] %0 : $Foo to $Bar, bb1, bb2 bb1(%1 : $Bar): %2 = unchecked_ref_cast %0 : $Foo to %1 : $Bar apply ...(..., %2) br ... bb2: ... This is not ownership SIL-safe, because we're re-using the original operand, which will have already been consumed at that point. The more immediate problem here is that this is actually not safe when combined with other optimizations. Suppose that after the speculative devirtualization, our function is inlined into another function where the operand was an upcast. Now we have this code: %0 = upcast ... checked_cast_br [exact] %0 : $Foo to $Bar, bb1, bb2 bb1(%1 : $Bar): %2 = unchecked_ref_cast %0 : $Foo to %1 : $Bar apply ...(..., %2) br ... bb2: ... The SILCombiner will simplify the unchecked_ref_cast of the upcast into an unchecked_ref_cast of the upcast's original operand. At this point, we have an unchecked_ref_cast between two unrelated class types. While this means the block bb1 is unreachable, we might perform further optimizations on it before we run the cast optimizer and delete it altogether. In particular, the devirtualizer follows chains of reference cast instructions, and it will get very confused if it finds this invalid cast between unrelated types. Fixes <rdar://problem/57712094>, <https://bugs.swift.org/browse/SR-11916>.
1 parent 0edf4a9 commit 07fb13f

File tree

3 files changed

+80
-23
lines changed

3 files changed

+80
-23
lines changed

‎lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,20 @@ static SILBasicBlock *cloneEdge(TermInst *TI, unsigned SuccIndex) {
8181
}
8282

8383
// A utility function for cloning the apply instruction.
84-
static FullApplySite CloneApply(FullApplySite AI, SILBuilder &Builder) {
84+
static FullApplySite CloneApply(FullApplySite AI, SILValue SelfArg,
85+
SILBuilder &Builder) {
8586
// Clone the Apply.
8687
Builder.setCurrentDebugScope(AI.getDebugScope());
8788
Builder.addOpenedArchetypeOperands(AI.getInstruction());
8889
auto Args = AI.getArguments();
8990
SmallVector<SILValue, 8> Ret(Args.size());
90-
for (unsigned i = 0, e = Args.size(); i != e; ++i)
91-
Ret[i] = Args[i];
91+
for (unsigned i = 0, e = Args.size(); i != e; ++i) {
92+
if (i == e - 1 && SelfArg) {
93+
Ret[i] = SelfArg;
94+
} else {
95+
Ret[i] = Args[i];
96+
}
97+
}
9298

9399
FullApplySite NAI;
94100

@@ -169,8 +175,8 @@ static FullApplySite speculateMonomorphicTarget(FullApplySite AI,
169175
SILValue DownCastedClassInstance = Iden->getArgument(0);
170176

171177
// Copy the two apply instructions into the two blocks.
172-
FullApplySite IdenAI = CloneApply(AI, IdenBuilder);
173-
FullApplySite VirtAI = CloneApply(AI, VirtBuilder);
178+
FullApplySite IdenAI = CloneApply(AI, DownCastedClassInstance, IdenBuilder);
179+
FullApplySite VirtAI = CloneApply(AI, SILValue(), VirtBuilder);
174180

175181
// See if Continue has a release on self as the instruction right after the
176182
// apply. If it exists, move it into position in the diamond.

‎test/SILOptimizer/devirt_speculate.swift

Lines changed: 69 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,44 +10,96 @@ public class Base {
1010
public init() {}
1111
public func foo() {}
1212
}
13+
14+
@_optimize(none)
15+
func blackHole<T : AnyObject>(_: T) {}
16+
1317
class Sub1 : Base {
14-
override func foo() {}
18+
override func foo() { blackHole(self) }
1519
}
1620
class Sub2 : Base {
17-
override func foo() {}
21+
override func foo() { blackHole(self) }
1822
}
1923
class Sub3 : Base {
20-
override func foo() {}
24+
override func foo() { blackHole(self) }
2125
}
2226
class Sub4 : Base {
23-
override func foo() {}
27+
override func foo() { blackHole(self) }
2428
}
2529
class Sub5 : Base {
26-
override func foo() {}
30+
override func foo() { blackHole(self) }
2731
}
2832
class Sub6 : Base {
29-
override func foo() {}
33+
override func foo() { blackHole(self) }
3034
}
3135
class Sub7 : Base {
32-
override func foo() {}
36+
override func foo() { blackHole(self) }
3337
}
3438
// CHECK: @$s16devirt_speculate28testMaxNumSpeculativeTargetsyyAA4BaseCF
35-
// CHECK: checked_cast_br [exact] %0 : $Base to Base
36-
// CHECK: checked_cast_br [exact] %0 : $Base to Sub1
37-
// CHECK: checked_cast_br [exact] %0 : $Base to Sub2
38-
// CHECK: checked_cast_br [exact] %0 : $Base to Sub3
39-
// CHECK: checked_cast_br [exact] %0 : $Base to Sub4
40-
// CHECK: checked_cast_br [exact] %0 : $Base to Sub5
41-
// CHECK: checked_cast_br [exact] %0 : $Base to Sub6
39+
// CHECK: bb0(%0 : $Base):
40+
// CHECK: checked_cast_br [exact] %0 : $Base to Base, bb2, bb3
41+
42+
// CHECK: bb2([[CASTED:%.*]]):
43+
// CHECK: br bb1
44+
45+
// CHECK: bb3:
46+
// CHECK: checked_cast_br [exact] %0 : $Base to Sub1, bb4, bb5
47+
48+
// CHECK: bb4([[CASTED:%.*]] : $Sub1):
49+
// CHECK: [[FN:%.*]] = function_ref @$s16devirt_speculate9blackHoleyyxRlzClF : $@convention(thin) <τ_0_0 where τ_0_0 : AnyObject> (@guaranteed τ_0_0) -> ()
50+
// CHECK: apply [[FN]]<Sub1>([[CASTED]])
51+
// CHECK: br bb1
52+
53+
// CHECK: bb5:
54+
// CHECK: checked_cast_br [exact] %0 : $Base to Sub2, bb6, bb7
55+
56+
// CHECK: bb6([[CASTED:%.*]] : $Sub2):
57+
// CHECK: [[FN:%.*]] = function_ref @$s16devirt_speculate9blackHoleyyxRlzClF : $@convention(thin) <τ_0_0 where τ_0_0 : AnyObject> (@guaranteed τ_0_0) -> ()
58+
// CHECK: apply [[FN]]<Sub2>([[CASTED]])
59+
// CHECK: br bb1
60+
61+
// CHECK: bb7:
62+
// CHECK: checked_cast_br [exact] %0 : $Base to Sub3, bb8, bb9
63+
64+
// CHECK: bb8([[CASTED:%.*]] : $Sub3):
65+
// CHECK: [[FN:%.*]] = function_ref @$s16devirt_speculate9blackHoleyyxRlzClF : $@convention(thin) <τ_0_0 where τ_0_0 : AnyObject> (@guaranteed τ_0_0) -> ()
66+
// CHECK: apply [[FN]]<Sub3>([[CASTED]])
67+
// CHECK: br bb1
68+
69+
// CHECK: bb9:
70+
// CHECK: checked_cast_br [exact] %0 : $Base to Sub4, bb10, bb11
71+
72+
// CHECK: bb10([[CASTED:%.*]] : $Sub4):
73+
// CHECK: [[FN:%.*]] = function_ref @$s16devirt_speculate9blackHoleyyxRlzClF : $@convention(thin) <τ_0_0 where τ_0_0 : AnyObject> (@guaranteed τ_0_0) -> ()
74+
// CHECK: apply [[FN]]<Sub4>([[CASTED]])
75+
// CHECK: br bb1
76+
77+
// CHECK: bb11:
78+
// CHECK: checked_cast_br [exact] %0 : $Base to Sub5, bb12, bb13
79+
80+
// CHECK: bb12([[CASTED:%.*]] : $Sub5):
81+
// CHECK: [[FN:%.*]] = function_ref @$s16devirt_speculate9blackHoleyyxRlzClF : $@convention(thin) <τ_0_0 where τ_0_0 : AnyObject> (@guaranteed τ_0_0) -> ()
82+
// CHECK: apply [[FN]]<Sub5>([[CASTED]])
83+
// CHECK: br bb1
84+
85+
// CHECK: bb13:
86+
// CHECK: checked_cast_br [exact] %0 : $Base to Sub6, bb14, bb15
87+
88+
// CHECK: bb14([[CASTED:%.*]] : $Sub6):
89+
// CHECK: [[FN:%.*]] = function_ref @$s16devirt_speculate9blackHoleyyxRlzClF : $@convention(thin) <τ_0_0 where τ_0_0 : AnyObject> (@guaranteed τ_0_0) -> ()
90+
// CHECK: apply [[FN]]<Sub6>([[CASTED]])
91+
// CHECK: br bb1
92+
93+
// CHECK: bb15:
4294
// CHECK-NOT: checked_cast_br
43-
// CHECK: %[[CM:[0-9]+]] = class_method %0 : $Base, #Base.foo!1 : (Base) -> () -> (), $@convention(method) (@guaranteed Base) -> ()
44-
// CHECK: apply %[[CM]](%0) : $@convention(method) (@guaranteed Base) -> ()
95+
// CHECK: %[[CM:[0-9]+]] = class_method %0 : $Base, #Base.foo!1 : (Base) -> () -> (), $@convention(method) (@guaranteed Base) -> ()
96+
// CHECK: apply %[[CM]](%0) : $@convention(method) (@guaranteed Base) -> ()
4597

4698
// YAML: Pass: sil-speculative-devirtualizer
4799
// YAML-NEXT: Name: sil.PartialSpecDevirt
48100
// YAML-NEXT: DebugLoc:
49101
// YAML-NEXT: File: {{.*}}/devirt_speculate.swift
50-
// YAML-NEXT: Line: 66
102+
// YAML-NEXT: Line: 118
51103
// YAML-NEXT: Column: 5
52104
// YAML-NEXT: Function: 'testMaxNumSpeculativeTargets(_:)'
53105
// YAML-NEXT: Args:

‎test/SILOptimizer/devirt_speculative.sil

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ bb0(%0: $Base2):
145145
// CHECK-NEXT: unreachable
146146
// CHECK: checked_cast_br
147147
// CHECK: function_ref @Sub_exit
148-
// CHECK-NEXT: unchecked_ref_cast
149148
// CHECK-NEXT: apply
150149
// CHECK-NEXT: unreachable
151150
sil hidden [noinline] @test_devirt_of_noreturn_function : $@convention(thin) (@owned Base) -> () {

0 commit comments

Comments
 (0)