Skip to content

Commit b5ad038

Browse files
committed
Fix SILCombine infinite iteration optimizing existentials.
Optimizing applies that take existential arguments is a complex process with multiple transformations. If the apply isn't rewritten, earlier transformations need to be undone. SILCombine is obviously the wrong place to implement type propagation, as I point out every time I need to fix it. Fixes <rdar://problem/49336444> SILCombine infinite loop.
1 parent 28ecdea commit b5ad038

File tree

2 files changed

+58
-2
lines changed

2 files changed

+58
-2
lines changed

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,10 @@ void SILCombiner::replaceWitnessMethodInst(
663663
// This function determines concrete type of an opened existential argument
664664
// using ProtocolConformanceAnalysis. The concrete type of the argument can be a
665665
// class, struct, or an enum.
666+
//
667+
// If some ConcreteOpenedExistentialInfo is returned, then new cast instructions
668+
// have already been added to Builder's tracking list. If the caller can't make
669+
// real progress then it must reset the Builder.
666670
Optional<ConcreteOpenedExistentialInfo>
667671
SILCombiner::buildConcreteOpenedExistentialInfoFromSoleConformingType(
668672
Operand &ArgOperand) {
@@ -998,9 +1002,15 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
9981002
});
9991003
}
10001004

1001-
if (!UpdatedArgs)
1005+
if (!UpdatedArgs) {
1006+
// Remove any new instructions created while attempting to optimize this
1007+
// apply. Since the apply was never rewritten, if they aren't removed here,
1008+
// they will be removed later as dead when visited by SILCombine, causing
1009+
// SILCombine to loop infinitely, creating and destroying the casts.
1010+
recursivelyDeleteTriviallyDeadInstructions(*Builder.getTrackingList());
1011+
Builder.getTrackingList()->clear();
10021012
return nullptr;
1003-
1013+
}
10041014
// Now create the new apply instruction.
10051015
SILBuilderWithScope ApplyBuilder(Apply.getInstruction(), BuilderCtx);
10061016
FullApplySite NewApply;
@@ -1132,6 +1142,9 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply) {
11321142
if (COEIs.empty())
11331143
return nullptr;
11341144

1145+
// At least one COEI is present, so cast instructions may already have been
1146+
// inserted. We must either rewrite the apply or delete the casts and reset
1147+
// the Builder's tracking list.
11351148
return createApplyWithConcreteType(Apply, COEIs, BuilderCtx);
11361149
}
11371150

test/SILOptimizer/existential_specializer_soletype.sil

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,46 @@ sil_vtable RC {
139139
sil_witness_table hidden RC: RP module simple {
140140
method #RP.getThres!1: <Self where Self : RP> (Self) -> () -> Int32 : @$s6simple2RCCAA2RPA2aDP8getThress5Int32VyFTW
141141
}
142+
143+
144+
// <rdar://problem/49336444> SILCombine infinite loop.
145+
//
146+
// Test a apply argument from an init_existential with a sole
147+
// conforming type. We currently bail on rewriting the apply because
148+
// it returns the same substituted type. Avoid infinitely iterating in
149+
// SILCombine due to repeatedly creating an destroying the same cast.
150+
public protocol BaseProtocol {
151+
func testProtocolMethod() -> Self
152+
}
153+
extension BaseProtocol {
154+
func testProtocolMethod() -> Self {
155+
return self
156+
}
157+
}
158+
159+
protocol SubProtocol : BaseProtocol {}
160+
161+
final class ClassImpl : SubProtocol {}
162+
163+
extension ClassImpl {
164+
final func testProtocolMethod() -> ClassImpl
165+
}
166+
167+
sil @testProtocolMethod : $@convention(method) <τ_0_0 where τ_0_0 : BaseProtocol> (@in_guaranteed τ_0_0) -> @out τ_0_0
168+
169+
// Verify that the optimization was not performance and that we don't hang as a result.
170+
// CHECK-LABEL: sil hidden @testConcreteInitExistential : $@convention(method) (@in SubProtocol) -> () {
171+
// CHECK: [[E:%.*]] = init_existential_addr %{{.*}} : $*SubProtocol, $@opened("{{.*}}") SubProtocol
172+
// CHECK: apply %{{.*}}<@opened("{{.*}}") SubProtocol>([[E]], %{{.*}}) : $@convention(method) <τ_0_0 where τ_0_0 : BaseProtocol> (@in_guaranteed τ_0_0) -> @out τ_0_0
173+
// CHECK-LABEL: } // end sil function 'testConcreteInitExistential'
174+
sil hidden @testConcreteInitExistential : $@convention(method) (@in SubProtocol) -> () {
175+
bb0(%0 : $*SubProtocol):
176+
%10 = open_existential_addr immutable_access %0 : $*SubProtocol to $*@opened("CA90348E-5376-11E9-8C51-ACDE48001122") SubProtocol
177+
%11 = alloc_stack $SubProtocol
178+
%15 = function_ref @testProtocolMethod : $@convention(method) <τ_0_0 where τ_0_0 : BaseProtocol> (@in_guaranteed τ_0_0) -> @out τ_0_0
179+
%16 = init_existential_addr %11 : $*SubProtocol, $@opened("CA90348E-5376-11E9-8C51-ACDE48001122") SubProtocol
180+
%17 = apply %15<@opened("CA90348E-5376-11E9-8C51-ACDE48001122") SubProtocol>(%16, %10) : $@convention(method) <τ_0_0 where τ_0_0 : BaseProtocol> (@in_guaranteed τ_0_0) -> @out τ_0_0
181+
dealloc_stack %11 : $*SubProtocol
182+
%80 = tuple ()
183+
return %80 : $()
184+
}

0 commit comments

Comments
 (0)