Skip to content

Commit 13e0b31

Browse files
committed
SILCombiner: createApplyWithConcreteType should not proceed if the 'apply' involves non-root opened archetypes
This optimization rewrites only the 'self' argument, and does not know how to substitute types in the users of the given apply instruction in case the underlying protocol method involved `Self`-dependent types. With SE-0309 in motion, the bail-out logic must be generalized to non-root opened archetypes.
1 parent 60770f1 commit 13e0b31

File tree

1 file changed

+53
-54
lines changed

1 file changed

+53
-54
lines changed

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -890,45 +890,56 @@ static bool canReplaceCopiedArg(FullApplySite Apply, SILValue Arg,
890890
return true;
891891
}
892892

893+
/// Determine if the result type or argument types of the given apply, except
894+
/// for the argument at \p SkipArgIdx, contain an opened archetype rooted
895+
/// on \p RootOA.
896+
static bool applyInvolvesOpenedArchetypeWithRoot(FullApplySite Apply,
897+
OpenedArchetypeType *RootOA,
898+
unsigned SkipArgIdx) {
899+
if (Apply.getType().getASTType()->hasOpenedExistentialWithRoot(RootOA)) {
900+
return true;
901+
}
902+
903+
const auto NumApplyArgs = Apply.getNumArguments();
904+
for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
905+
if (Idx == SkipArgIdx)
906+
continue;
907+
if (Apply.getArgument(Idx)
908+
->getType()
909+
.getASTType()
910+
->hasOpenedExistentialWithRoot(RootOA)) {
911+
return true;
912+
}
913+
}
914+
915+
return false;
916+
}
917+
893918
// Check the legal conditions under which a Arg parameter (specified as ArgIdx)
894919
// can be replaced with a concrete type. Concrete type info is passed as CEI
895920
// argument.
896921
bool SILCombiner::canReplaceArg(FullApplySite Apply,
897922
const OpenedArchetypeInfo &OAI,
898923
const ConcreteExistentialInfo &CEI,
899924
unsigned ArgIdx) {
900-
901-
// Don't specialize apply instructions that return the callee's Arg type,
902-
// because this optimization does not know how to substitute types in the
903-
// users of this apply. In the function type substitution below, all
904-
// references to OpenedArchetype will be substituted. So walk to type to
905-
// find all possible references, such as returning Optional<Arg>.
906-
if (Apply.getType().getASTType().findIf(
907-
[&OAI](Type t) -> bool { return t->isEqual(OAI.OpenedArchetype); })) {
908-
return false;
909-
}
910-
// Bail out if any other arguments or indirect result that refer to the
911-
// OpenedArchetype. The following optimization substitutes all occurrences
912-
// of OpenedArchetype in the function signature, but will only rewrite the
913-
// Arg operand.
925+
// Don't specialize apply instructions if the result type references
926+
// OpenedArchetype, because this optimization does not know how to substitute
927+
// types in the users of this apply. In the function type substitution below,
928+
// all references to OpenedArchetype will be substituted. So walk the type to
929+
// find all possible references, such as returning Optional<OpenedArchetype>.
930+
// The same holds for other arguments or indirect result that refer to the
931+
// OpenedArchetype, because the following optimization will rewrite only the
932+
// argument at ArgIdx.
914933
//
915934
// Note that the language does not allow Self to occur in contravariant
916935
// position. However, SIL does allow this and it can happen as a result of
917936
// upstream transformations. Since this is bail-out logic, it must handle
918937
// all verifiable SIL.
919-
920-
// This bailout check is also needed for non-Self arguments [including Self].
921-
unsigned NumApplyArgs = Apply.getNumArguments();
922-
for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
923-
if (Idx == ArgIdx)
924-
continue;
925-
if (Apply.getArgument(Idx)->getType().getASTType().findIf(
926-
[&OAI](Type t) -> bool {
927-
return t->isEqual(OAI.OpenedArchetype);
928-
})) {
929-
return false;
930-
}
938+
if (applyInvolvesOpenedArchetypeWithRoot(Apply, OAI.OpenedArchetype,
939+
ArgIdx)) {
940+
return false;
931941
}
942+
932943
// If the convention is mutating, then the existential must have been
933944
// initialized by copying the concrete value (regardless of whether
934945
// CEI.isConcreteValueCopied is true). Replacing the existential address with
@@ -1022,37 +1033,24 @@ SILValue SILCombiner::canCastArg(FullApplySite Apply,
10221033
!CEI.ConcreteValue->getType().isAddress())
10231034
return SILValue();
10241035

1025-
// Don't specialize apply instructions that return the callee's Arg type,
1026-
// because this optimization does not know how to substitute types in the
1027-
// users of this apply. In the function type substitution below, all
1028-
// references to OpenedArchetype will be substituted. So walk to type to
1029-
// find all possible references, such as returning Optional<Arg>.
1030-
if (Apply.getType().getASTType().findIf(
1031-
[&OAI](Type t) -> bool { return t->isEqual(OAI.OpenedArchetype); })) {
1032-
return SILValue();
1033-
}
1034-
// Bail out if any other arguments or indirect result that refer to the
1035-
// OpenedArchetype. The following optimization substitutes all occurrences
1036-
// of OpenedArchetype in the function signature, but will only rewrite the
1037-
// Arg operand.
1036+
// Don't specialize apply instructions if the result type references
1037+
// OpenedArchetype, because this optimization does not know how to substitute
1038+
// types in the users of this apply. In the function type substitution below,
1039+
// all references to OpenedArchetype will be substituted. So walk the type to
1040+
// find all possible references, such as returning Optional<OpenedArchetype>.
1041+
// The same holds for other arguments or indirect result that refer to the
1042+
// OpenedArchetype, because the following optimization will rewrite only the
1043+
// argument at ArgIdx.
10381044
//
10391045
// Note that the language does not allow Self to occur in contravariant
10401046
// position. However, SIL does allow this and it can happen as a result of
10411047
// upstream transformations. Since this is bail-out logic, it must handle
10421048
// all verifiable SIL.
1043-
1044-
// This bailout check is also needed for non-Self arguments [including Self].
1045-
unsigned NumApplyArgs = Apply.getNumArguments();
1046-
for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
1047-
if (Idx == ArgIdx)
1048-
continue;
1049-
if (Apply.getArgument(Idx)->getType().getASTType().findIf(
1050-
[&OAI](Type t) -> bool {
1051-
return t->isEqual(OAI.OpenedArchetype);
1052-
})) {
1053-
return SILValue();
1054-
}
1049+
if (applyInvolvesOpenedArchetypeWithRoot(Apply, OAI.OpenedArchetype,
1050+
ArgIdx)) {
1051+
return SILValue();
10551052
}
1053+
10561054
return Builder.createUncheckedAddrCast(
10571055
Apply.getLoc(), Apply.getArgument(ArgIdx), CEI.ConcreteValue->getType());
10581056
}
@@ -1263,10 +1261,11 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
12631261
/// %existential = alloc_stack $Protocol
12641262
/// %value = init_existential_addr %existential : $Concrete
12651263
/// copy_addr ... to %value
1266-
/// %witness = witness_method $@opened
1267-
/// apply %witness<T : Protocol>(%existential)
1264+
/// %opened = open_existential_addr %existential
1265+
/// %witness = witness_method $@opened(...) Protocol
1266+
/// apply %witness<$@opened(...) Protocol>(%opened)
12681267
///
1269-
/// ==> apply %witness<Concrete : Protocol>(%existential)
1268+
/// ==> apply %witness<$Concrete>(%existential)
12701269
SILInstruction *
12711270
SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply,
12721271
WitnessMethodInst *WMI) {

0 commit comments

Comments
 (0)