Skip to content

Commit 7a9b05b

Browse files
committed
[sil-devirtualizer] Fix some bugs in the devirtualizer.
- Don't crash if a class_method instruction could not be devirtualized. - Improve devirtualization of methods with generic parameters and using dependent types. - Fix a bug in isBindableToSuperclassOf, uncovered while fixing the original bug reported in SR-1206. This bug could lead in certain cases to invocations of a wrong method from the base class, instead of using a method from a derived class. rdar://25891588 and SR-1206
1 parent 3af0796 commit 7a9b05b

File tree

4 files changed

+175
-24
lines changed

4 files changed

+175
-24
lines changed

lib/AST/Type.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,8 +1804,9 @@ static bool isBindableTo(Type a, Type b, LazyResolver *resolver) {
18041804
== substSubs[subi].getConformances().size());
18051805
for (unsigned conformancei :
18061806
indices(origSubs[subi].getConformances())) {
1807-
if (origSubs[subi].getConformances()[conformancei]
1808-
!= substSubs[subi].getConformances()[conformancei])
1807+
if (origSubs[subi].getConformances()[conformancei].isConcrete() &&
1808+
origSubs[subi].getConformances()[conformancei] !=
1809+
substSubs[subi].getConformances()[conformancei])
18091810
return false;
18101811
}
18111812
}

lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -509,10 +509,11 @@ static bool tryToSpeculateTarget(FullApplySite AI,
509509
return true;
510510
}
511511
auto NewInstPair = tryDevirtualizeClassMethod(AI, SubTypeValue);
512-
assert(NewInstPair.first && "Expected to be able to devirtualize apply!");
513-
replaceDeadApply(AI, NewInstPair.first);
514-
515-
return true;
512+
if (NewInstPair.first) {
513+
replaceDeadApply(AI, NewInstPair.first);
514+
return true;
515+
}
516+
return Changed;
516517
}
517518

518519
namespace {

lib/SILOptimizer/Utils/Devirtualize.cpp

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -389,17 +389,30 @@ getSubstitutionsForCallee(SILModule &M, CanSILFunctionType GenCalleeType,
389389
CanSILFunctionType AIGenCalleeType =
390390
AI.getCallee()->getType().castTo<SILFunctionType>();
391391

392-
CanType AISelfClass = AIGenCalleeType->getSelfParameter().getType();
393-
394392
unsigned NextMethodParamIdx = 0;
395393
unsigned NumMethodParams = 0;
396394
if (AIGenCalleeType->isPolymorphic()) {
397-
NextMethodParamIdx = 0;
398-
// Generic parameters of the method start after generic parameters
399-
// of the instance class.
400-
if (auto AISelfClassSig =
401-
AISelfClass.getClassBound()->getGenericSignature()) {
402-
NextMethodParamIdx = AISelfClassSig->getGenericParams().size();
395+
NextMethodParamIdx = AISubs.size();
396+
if (auto AIMethodSig = AI.getOrigCalleeType()->getGenericSignature()) {
397+
// Find out if the apply instruction contains any substitutions for
398+
// a method itself, not for the class where it is declared.
399+
// If there are any such parameters, remember where they start
400+
// in the substitutions list.
401+
auto InnermostGenericParams = AIMethodSig->getInnermostGenericParams();
402+
auto InnermostGenericParamsNum = InnermostGenericParams.size();
403+
if (InnermostGenericParamsNum != AIMethodSig->getGenericParams().size()) {
404+
auto Depth = InnermostGenericParams[0]->getDepth();
405+
for (NextMethodParamIdx = 0; NextMethodParamIdx < AISubs.size();
406+
++NextMethodParamIdx) {
407+
if (auto SubstTy = dyn_cast<SubstitutedType>(
408+
AISubs[NextMethodParamIdx].getReplacement().getPointer())) {
409+
if (auto *GenParamTy = dyn_cast<GenericTypeParamType>(
410+
SubstTy->getOriginal().getPointer()))
411+
if (GenParamTy->getDepth() == Depth)
412+
break;
413+
}
414+
}
415+
}
403416
}
404417
NumMethodParams = AISubs.size() - NextMethodParamIdx;
405418
}
@@ -477,37 +490,42 @@ bool swift::canDevirtualizeClassMethod(FullApplySite AI,
477490
return false;
478491
}
479492

493+
// Type of the actual function to be called.
480494
CanSILFunctionType GenCalleeType = F->getLoweredFunctionType();
481495

482-
auto Subs = getSubstitutionsForCallee(Mod, GenCalleeType,
483-
ClassOrMetatypeType, AI);
496+
// Type of the actual function to be called with substitutions applied.
497+
CanSILFunctionType SubstCalleeType = GenCalleeType;
484498

485499
// For polymorphic functions, bail if the number of substitutions is
486500
// not the same as the number of expected generic parameters.
487501
if (GenCalleeType->isPolymorphic()) {
502+
// First, find proper list of substitutions for the concrete
503+
// method to be called.
504+
auto Subs = getSubstitutionsForCallee(Mod, GenCalleeType,
505+
ClassOrMetatypeType, AI);
506+
488507
auto GenericSig = GenCalleeType->getGenericSignature();
489508
// Get the number of expected generic parameters, which
490509
// is a sum of the number of explicit generic parameters
491510
// and the number of their recursive member types exposed
492511
// through protocol requirements.
493512
auto DepTypes = GenericSig->getAllDependentTypes();
494-
unsigned ExpectedGenParamsNum = 0;
513+
unsigned ExpectedSubsNum = 0;
495514

496515
for (auto DT: DepTypes) {
497516
(void)DT;
498-
ExpectedGenParamsNum++;
517+
ExpectedSubsNum++;
499518
}
500519

501-
if (ExpectedGenParamsNum != Subs.size())
520+
if (ExpectedSubsNum != Subs.size()) {
502521
return false;
503-
}
522+
}
504523

505-
// Check if the optimizer knows how to cast the return type.
506-
CanSILFunctionType SubstCalleeType = GenCalleeType;
507-
if (GenCalleeType->isPolymorphic())
508524
SubstCalleeType =
509525
GenCalleeType->substGenericArgs(Mod, Mod.getSwiftModule(), Subs);
526+
}
510527

528+
// Check if the optimizer knows how to cast the return type.
511529
SILType ReturnType = SubstCalleeType->getSILResult();
512530

513531
if (!canCastValueToABICompatibleType(Mod, ReturnType, AI.getType()))

test/SILOptimizer/devirt_unbound_generic.swift

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-sil -O -emit-sil %s | FileCheck %s
1+
// RUN: %target-swift-frontend -emit-sil -O %s | FileCheck %s
22

33
// We used to crash on this when trying to devirtualize t.boo(a, 1),
44
// because it is an "apply" with unbound generic arguments and
@@ -73,3 +73,134 @@ func test3<T>(_ d: Derived<T>) {
7373
public func doTest3<T>(_ t:T) {
7474
test3(Derived<T>())
7575
}
76+
77+
78+
public protocol ProtocolWithAssocType {
79+
associatedtype Element
80+
}
81+
82+
private class CP<Base: ProtocolWithAssocType> {
83+
var value: Base.Element
84+
init(_ v: Base.Element) {
85+
value = v
86+
}
87+
88+
func getCount() -> Int32 {
89+
return 1
90+
}
91+
}
92+
93+
private class Base1: ProtocolWithAssocType {
94+
typealias Element = Int32
95+
}
96+
97+
98+
private class Base2<T>: ProtocolWithAssocType {
99+
typealias Element = Int32
100+
}
101+
102+
private class CP2: CP<Base2<Int>> {
103+
init() {
104+
super.init(1)
105+
}
106+
107+
override func getCount() -> Int32 {
108+
return 2
109+
}
110+
}
111+
112+
private class CP3: CP<Base2<Int>> {
113+
init() {
114+
super.init(1)
115+
}
116+
117+
override func getCount() -> Int32 {
118+
return 3
119+
}
120+
}
121+
122+
public class CC<CT> {
123+
func next() -> CT? {
124+
return nil
125+
}
126+
}
127+
128+
public protocol QQ {
129+
associatedtype Base: PP
130+
}
131+
132+
public protocol PP {
133+
associatedtype Element
134+
}
135+
136+
internal class D<DT: QQ> : CC<DT.Base.Element> {
137+
override func next() -> DT.Base.Element? {
138+
return nil
139+
}
140+
}
141+
142+
public struct S: PP {
143+
public typealias Element = Int32
144+
}
145+
146+
final public class E: QQ {
147+
public typealias Base = S
148+
}
149+
150+
// Check that c.next() inside test4 gets completely devirtualized.
151+
// CHECK-LABEL: sil @{{.*}}test4
152+
// CHECK-NOT: class_method
153+
// CHECK: return
154+
public func test4() -> Int32? {
155+
let c: CC<Int32> = D<E>();
156+
return c.next()
157+
}
158+
159+
public func test5() -> Int32? {
160+
return testDevirt(D<E>())
161+
}
162+
163+
// Check that testDevirt is specialized and uses speculative devirtualization.
164+
// CHECK-LABEL: sil shared [noinline] @{{.*}}testDevirt
165+
// CHECK: checked_cast_br [exact] %{{.*}} : $CC<Int32> to $CC<Int32>
166+
// CHECK: class_method
167+
// CHECK: }
168+
@inline(never)
169+
public func testDevirt<T>(_ c: CC<T>) -> T? {
170+
return c.next()
171+
}
172+
173+
// The compiler used to crash on this code, because of
174+
// generic types involved in the devirtualization.
175+
//
176+
// rdar://25891588
177+
//
178+
// CHECK-LABEL: sil private [noinline] @{{.*}}test6
179+
// CHECK-NOT: class_method
180+
// CHECK-NOT: checked_cast_br
181+
// CHECK-NOT: class_method
182+
// CHECK: }
183+
@inline(never)
184+
private func test6<T: ProtocolWithAssocType>(_ c: CP<T>) -> T.Element {
185+
return c.value
186+
}
187+
188+
public func doTest6() {
189+
test6(CP<Base1>(1))
190+
}
191+
192+
// CHECK-LABEL: sil private [noinline] @{{.*}}test7
193+
// CHECK-NOT: class_method
194+
// CHECK: checked_cast_br
195+
// CHECK-NOT: class_method
196+
// CHECK: }
197+
@inline(never)
198+
private func test7<T: ProtocolWithAssocType>(_ c: CP<T>) -> Int32 {
199+
return c.getCount()
200+
}
201+
202+
public func doTest7() {
203+
test7(CP2())
204+
}
205+
206+

0 commit comments

Comments
 (0)