Skip to content

Commit ac74b84

Browse files
authored
Merge pull request swiftlang#58513 from AnthonyLatsis/se-309-sil-opt
SE-0309: SILOptimizer fixes & reenable executable tests
2 parents de47a77 + 5dc4834 commit ac74b84

File tree

13 files changed

+559
-255
lines changed

13 files changed

+559
-255
lines changed

include/swift/AST/Types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5918,6 +5918,9 @@ class OpenedArchetypeType final : public ArchetypeType,
59185918
LayoutConstraint layout);
59195919
};
59205920
BEGIN_CAN_TYPE_WRAPPER(OpenedArchetypeType, ArchetypeType)
5921+
CanOpenedArchetypeType getRoot() const {
5922+
return CanOpenedArchetypeType(getPointer()->getRoot());
5923+
}
59215924
END_CAN_TYPE_WRAPPER(OpenedArchetypeType, ArchetypeType)
59225925

59235926
/// An archetype that represents an opaque element of a type sequence in context.

include/swift/SILOptimizer/Utils/Existential.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace swift {
3333
/// When successfull, ConcreteExistentialInfo can be used to determine the
3434
/// concrete type of the opened existential.
3535
struct OpenedArchetypeInfo {
36-
ArchetypeType *OpenedArchetype = nullptr;
36+
OpenedArchetypeType *OpenedArchetype = nullptr;
3737
// The opened value.
3838
SingleValueInstruction *OpenedArchetypeValue;
3939
// The existential value.

lib/AST/Type.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ Type QueryTypeSubstitutionMap::operator()(SubstitutableType *type) const {
6565

6666
Type
6767
QueryTypeSubstitutionMapOrIdentity::operator()(SubstitutableType *type) const {
68+
// FIXME: Type::subst should not be pass in non-root archetypes.
69+
// Consider only root archetypes.
70+
if (auto *archetype = dyn_cast<ArchetypeType>(type)) {
71+
if (!archetype->isRoot())
72+
return Type();
73+
}
74+
6875
auto key = type->getCanonicalType()->castTo<SubstitutableType>();
6976
auto known = substitutions.find(key);
7077
if (known != substitutions.end() && known->second)

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,7 +2551,7 @@ CanSILFunctionType swift::buildSILFunctionThunkType(
25512551
auto archetypeVisitor = [&](CanType t) {
25522552
if (auto archetypeTy = dyn_cast<ArchetypeType>(t)) {
25532553
if (auto opened = dyn_cast<OpenedArchetypeType>(archetypeTy)) {
2554-
const auto root = cast<OpenedArchetypeType>(CanType(opened->getRoot()));
2554+
const auto root = opened.getRoot();
25552555
assert((openedExistential == CanArchetypeType() ||
25562556
openedExistential == root) &&
25572557
"one too many open existentials");
@@ -2582,18 +2582,16 @@ CanSILFunctionType swift::buildSILFunctionThunkType(
25822582
}
25832583

25842584
auto substTypeHelper = [&](SubstitutableType *type) -> Type {
2585+
// FIXME: Type::subst should not pass in non-root archetypes.
2586+
// Consider only root archetypes.
2587+
if (auto *archetype = dyn_cast<ArchetypeType>(type)) {
2588+
if (!archetype->isRoot())
2589+
return Type();
2590+
}
2591+
25852592
if (CanType(type) == openedExistential)
25862593
return newArchetype;
25872594

2588-
// If a nested archetype is rooted on our opened existential, fail:
2589-
// Type::subst attempts to substitute the parent of a nested archetype
2590-
// only if it fails to find a replacement for the nested one.
2591-
if (auto *opened = dyn_cast<OpenedArchetypeType>(type)) {
2592-
if (openedExistential->isEqual(opened->getRoot())) {
2593-
return nullptr;
2594-
}
2595-
}
2596-
25972595
return Type(type).subst(contextSubs);
25982596
};
25992597
auto substConformanceHelper =

lib/SIL/IR/SILInstructions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static void collectDependentTypeInfo(
5656
return;
5757
Ty.visit([&](CanType t) {
5858
if (const auto opened = dyn_cast<OpenedArchetypeType>(t)) {
59-
const auto root = cast<OpenedArchetypeType>(CanType(opened->getRoot()));
59+
const auto root = opened.getRoot();
6060

6161
// Add this root opened archetype if it was not seen yet.
6262
// We don't use a set here, because the number of open archetypes

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,10 +1469,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
14691469
"Operand is of an ArchetypeType that does not exist in the "
14701470
"Caller's generic param list.");
14711471
if (auto OpenedA = getOpenedArchetypeOf(A)) {
1472-
const auto root =
1473-
cast<OpenedArchetypeType>(CanType(OpenedA->getRoot()));
14741472
auto *openingInst =
1475-
F->getModule().getRootOpenedArchetypeDefInst(root, F);
1473+
F->getModule().getRootOpenedArchetypeDefInst(OpenedA.getRoot(), F);
14761474
require(I == nullptr || openingInst == I ||
14771475
properlyDominates(openingInst, I),
14781476
"Use of an opened archetype should be dominated by a "
@@ -1605,7 +1603,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
16051603
require(isArchetypeValidInFunction(A, AI->getFunction()),
16061604
"Archetype to be substituted must be valid in function.");
16071605

1608-
const auto root = cast<OpenedArchetypeType>(CanType(A->getRoot()));
1606+
const auto root = A.getRoot();
16091607

16101608
// Collect all root opened archetypes used in the substitutions list.
16111609
FoundRootOpenedArchetypes.insert(root);
@@ -4074,10 +4072,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
40744072
Ty.visit([&](CanType t) {
40754073
SILValue Def;
40764074
if (const auto archetypeTy = dyn_cast<OpenedArchetypeType>(t)) {
4077-
const auto root =
4078-
cast<OpenedArchetypeType>(CanType(archetypeTy->getRoot()));
4079-
Def = I->getModule().getRootOpenedArchetypeDefInst(root,
4080-
I->getFunction());
4075+
Def = I->getModule().getRootOpenedArchetypeDefInst(
4076+
archetypeTy.getRoot(), I->getFunction());
40814077
require(Def, "Root opened archetype should be registered in SILModule");
40824078
} else if (t->hasDynamicSelfType()) {
40834079
require(I->getFunction()->hasSelfParam() ||

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -920,45 +920,56 @@ static bool canReplaceCopiedArg(FullApplySite Apply, SILValue Arg,
920920
return true;
921921
}
922922

923+
/// Determine if the result type or argument types of the given apply, except
924+
/// for the argument at \p SkipArgIdx, contain an opened archetype rooted
925+
/// on \p RootOA.
926+
static bool applyInvolvesOpenedArchetypeWithRoot(FullApplySite Apply,
927+
OpenedArchetypeType *RootOA,
928+
unsigned SkipArgIdx) {
929+
if (Apply.getType().getASTType()->hasOpenedExistentialWithRoot(RootOA)) {
930+
return true;
931+
}
932+
933+
const auto NumApplyArgs = Apply.getNumArguments();
934+
for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
935+
if (Idx == SkipArgIdx)
936+
continue;
937+
if (Apply.getArgument(Idx)
938+
->getType()
939+
.getASTType()
940+
->hasOpenedExistentialWithRoot(RootOA)) {
941+
return true;
942+
}
943+
}
944+
945+
return false;
946+
}
947+
923948
// Check the legal conditions under which a Arg parameter (specified as ArgIdx)
924949
// can be replaced with a concrete type. Concrete type info is passed as CEI
925950
// argument.
926951
bool SILCombiner::canReplaceArg(FullApplySite Apply,
927952
const OpenedArchetypeInfo &OAI,
928953
const ConcreteExistentialInfo &CEI,
929954
unsigned ArgIdx) {
930-
931-
// Don't specialize apply instructions that return the callee's Arg type,
932-
// because this optimization does not know how to substitute types in the
933-
// users of this apply. In the function type substitution below, all
934-
// references to OpenedArchetype will be substituted. So walk to type to
935-
// find all possible references, such as returning Optional<Arg>.
936-
if (Apply.getType().getASTType().findIf(
937-
[&OAI](Type t) -> bool { return t->isEqual(OAI.OpenedArchetype); })) {
938-
return false;
939-
}
940-
// Bail out if any other arguments or indirect result that refer to the
941-
// OpenedArchetype. The following optimization substitutes all occurrences
942-
// of OpenedArchetype in the function signature, but will only rewrite the
943-
// Arg operand.
955+
// Don't specialize apply instructions if the result type references
956+
// OpenedArchetype, because this optimization does not know how to substitute
957+
// types in the users of this apply. In the function type substitution below,
958+
// all references to OpenedArchetype will be substituted. So walk the type to
959+
// find all possible references, such as returning Optional<OpenedArchetype>.
960+
// The same holds for other arguments or indirect result that refer to the
961+
// OpenedArchetype, because the following optimization will rewrite only the
962+
// argument at ArgIdx.
944963
//
945964
// Note that the language does not allow Self to occur in contravariant
946965
// position. However, SIL does allow this and it can happen as a result of
947966
// upstream transformations. Since this is bail-out logic, it must handle
948967
// all verifiable SIL.
949-
950-
// This bailout check is also needed for non-Self arguments [including Self].
951-
unsigned NumApplyArgs = Apply.getNumArguments();
952-
for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
953-
if (Idx == ArgIdx)
954-
continue;
955-
if (Apply.getArgument(Idx)->getType().getASTType().findIf(
956-
[&OAI](Type t) -> bool {
957-
return t->isEqual(OAI.OpenedArchetype);
958-
})) {
959-
return false;
960-
}
968+
if (applyInvolvesOpenedArchetypeWithRoot(Apply, OAI.OpenedArchetype,
969+
ArgIdx)) {
970+
return false;
961971
}
972+
962973
// If the convention is mutating, then the existential must have been
963974
// initialized by copying the concrete value (regardless of whether
964975
// CEI.isConcreteValueCopied is true). Replacing the existential address with
@@ -1052,37 +1063,24 @@ SILValue SILCombiner::canCastArg(FullApplySite Apply,
10521063
!CEI.ConcreteValue->getType().isAddress())
10531064
return SILValue();
10541065

1055-
// Don't specialize apply instructions that return the callee's Arg type,
1056-
// because this optimization does not know how to substitute types in the
1057-
// users of this apply. In the function type substitution below, all
1058-
// references to OpenedArchetype will be substituted. So walk to type to
1059-
// find all possible references, such as returning Optional<Arg>.
1060-
if (Apply.getType().getASTType().findIf(
1061-
[&OAI](Type t) -> bool { return t->isEqual(OAI.OpenedArchetype); })) {
1062-
return SILValue();
1063-
}
1064-
// Bail out if any other arguments or indirect result that refer to the
1065-
// OpenedArchetype. The following optimization substitutes all occurrences
1066-
// of OpenedArchetype in the function signature, but will only rewrite the
1067-
// Arg operand.
1066+
// Don't specialize apply instructions if the result type references
1067+
// OpenedArchetype, because this optimization does not know how to substitute
1068+
// types in the users of this apply. In the function type substitution below,
1069+
// all references to OpenedArchetype will be substituted. So walk the type to
1070+
// find all possible references, such as returning Optional<OpenedArchetype>.
1071+
// The same holds for other arguments or indirect result that refer to the
1072+
// OpenedArchetype, because the following optimization will rewrite only the
1073+
// argument at ArgIdx.
10681074
//
10691075
// Note that the language does not allow Self to occur in contravariant
10701076
// position. However, SIL does allow this and it can happen as a result of
10711077
// upstream transformations. Since this is bail-out logic, it must handle
10721078
// all verifiable SIL.
1073-
1074-
// This bailout check is also needed for non-Self arguments [including Self].
1075-
unsigned NumApplyArgs = Apply.getNumArguments();
1076-
for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
1077-
if (Idx == ArgIdx)
1078-
continue;
1079-
if (Apply.getArgument(Idx)->getType().getASTType().findIf(
1080-
[&OAI](Type t) -> bool {
1081-
return t->isEqual(OAI.OpenedArchetype);
1082-
})) {
1083-
return SILValue();
1084-
}
1079+
if (applyInvolvesOpenedArchetypeWithRoot(Apply, OAI.OpenedArchetype,
1080+
ArgIdx)) {
1081+
return SILValue();
10851082
}
1083+
10861084
return Builder.createUncheckedAddrCast(
10871085
Apply.getLoc(), Apply.getArgument(ArgIdx), CEI.ConcreteValue->getType());
10881086
}
@@ -1293,10 +1291,11 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
12931291
/// %existential = alloc_stack $Protocol
12941292
/// %value = init_existential_addr %existential : $Concrete
12951293
/// copy_addr ... to %value
1296-
/// %witness = witness_method $@opened
1297-
/// apply %witness<T : Protocol>(%existential)
1294+
/// %opened = open_existential_addr %existential
1295+
/// %witness = witness_method $@opened(...) Protocol
1296+
/// apply %witness<$@opened(...) Protocol>(%opened)
12981297
///
1299-
/// ==> apply %witness<Concrete : Protocol>(%existential)
1298+
/// ==> apply %witness<$Concrete>(%existential)
13001299
SILInstruction *
13011300
SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply,
13021301
WitnessMethodInst *WMI) {

lib/SILOptimizer/Utils/Devirtualize.cpp

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,9 +1094,10 @@ static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
10941094
return false;
10951095
}
10961096

1097-
// FIXME: devirtualizeWitnessMethod below does not support cases with
1098-
// covariant 'Self' nested inside a collection type,
1099-
// like '[Self]' or '[* : Self]'.
1097+
// FIXME: devirtualizeWitnessMethod does not support cases with covariant
1098+
// 'Self'-rooted type parameters nested inside a collection type, like
1099+
// '[Self]' or '[* : Self.A]', because it doesn't know how to deal with
1100+
// associated collection upcasts.
11001101
const Type interfaceTy = wmi->getMember()
11011102
.getDecl()
11021103
->getInterfaceType()
@@ -1107,35 +1108,35 @@ static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
11071108
if (!interfaceTy->hasTypeParameter())
11081109
return true;
11091110

1110-
class HasSelfNestedInsideCollection final : public TypeWalker {
1111-
unsigned CollectionDepth;
1111+
auto *const selfGP = wmi->getLookupProtocol()->getProtocolSelfType();
1112+
auto isSelfRootedTypeParameter = [selfGP](Type T) -> bool {
1113+
if (!T->hasTypeParameter())
1114+
return false;
11121115

1113-
public:
1114-
Action walkToTypePre(Type T) override {
1115-
if (!T->hasTypeParameter())
1116-
return Action::SkipChildren;
1116+
if (T->isTypeParameter()) {
1117+
return T->getRootGenericParam()->isEqual(selfGP);
1118+
}
11171119

1118-
if (auto *GP = T->getAs<GenericTypeParamType>()) {
1119-
// Only 'Self' will have zero depth in the type of a requirement.
1120-
if (GP->getDepth() == 0 && CollectionDepth)
1121-
return Action::Stop;
1122-
}
1120+
return false;
1121+
};
11231122

1124-
if (T->isArray() || T->isDictionary())
1125-
++CollectionDepth;
1123+
return !interfaceTy.findIf([&](Type T) -> bool {
1124+
if (!T->hasTypeParameter())
1125+
return false;
11261126

1127-
return Action::Continue;
1127+
if (T->isArray() || T->isDictionary()) {
1128+
return T.findIf(isSelfRootedTypeParameter);
11281129
}
11291130

1130-
Action walkToTypePost(Type T) override {
1131-
if (T->isArray() || T->isDictionary())
1132-
--CollectionDepth;
1133-
1134-
return Action::Continue;
1131+
if (auto *FT = T->getAs<FunctionType>()) {
1132+
for (const auto &Param : FT->getParams()) {
1133+
if (Param.isVariadic() && T.findIf(isSelfRootedTypeParameter))
1134+
return true;
1135+
}
11351136
}
1136-
};
11371137

1138-
return !interfaceTy.walk(HasSelfNestedInsideCollection());
1138+
return false;
1139+
});
11391140
}
11401141

11411142
/// In the cases where we can statically determine the function that

lib/SILOptimizer/Utils/Existential.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,13 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) {
220220
}
221221
}
222222
if (auto *Open = dyn_cast<OpenExistentialAddrInst>(openedVal)) {
223-
OpenedArchetype = Open->getType().castTo<ArchetypeType>();
223+
OpenedArchetype = Open->getType().castTo<OpenedArchetypeType>();
224224
OpenedArchetypeValue = Open;
225225
ExistentialValue = Open->getOperand();
226226
return;
227227
}
228228
if (auto *Open = dyn_cast<OpenExistentialRefInst>(openedVal)) {
229-
OpenedArchetype = Open->getType().castTo<ArchetypeType>();
229+
OpenedArchetype = Open->getType().castTo<OpenedArchetypeType>();
230230
OpenedArchetypeValue = Open;
231231
ExistentialValue = Open->getOperand();
232232
return;
@@ -235,7 +235,7 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) {
235235
auto Ty = Open->getType().getASTType();
236236
while (auto Metatype = dyn_cast<MetatypeType>(Ty))
237237
Ty = Metatype.getInstanceType();
238-
OpenedArchetype = cast<ArchetypeType>(Ty);
238+
OpenedArchetype = cast<OpenedArchetypeType>(Ty);
239239
OpenedArchetypeValue = Open;
240240
ExistentialValue = Open->getOperand();
241241
}

0 commit comments

Comments
 (0)