Skip to content

Commit eb45355

Browse files
slavapestovslavapestov
authored
Revert "SILCombiner: Clean up the concrete -> existential peephole a bit"
1 parent 9bc47ed commit eb45355

File tree

2 files changed

+69
-81
lines changed

2 files changed

+69
-81
lines changed

‎lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -692,41 +692,54 @@ SILCombiner::createApplyWithConcreteType(FullApplySite AI,
692692
// replaced by a concrete type.
693693
SmallVector<Substitution, 8> Substitutions;
694694
for (auto Subst : AI.getSubstitutions()) {
695-
auto NewSubst = Subst.subst(
696-
AI.getModule().getSwiftModule(),
697-
[&](SubstitutableType *type) -> Type {
698-
if (type == OpenedArchetype)
699-
return ConcreteType;
700-
return type;
701-
},
702-
[&](Type origTy, Type substTy, ProtocolType *protoType)
703-
-> Optional<ProtocolConformanceRef> {
704-
assert(origTy->isEqual(OpenedArchetype));
705-
return Conformance;
706-
});
707-
Substitutions.push_back(NewSubst);
695+
auto *A = Subst.getReplacement()->getAs<ArchetypeType>();
696+
if (A && A == OpenedArchetype) {
697+
auto Conformances = AI.getModule().getASTContext()
698+
.AllocateUninitialized<ProtocolConformanceRef>(1);
699+
Conformances[0] = Conformance;
700+
Substitution NewSubst(ConcreteType, Conformances);
701+
Substitutions.push_back(NewSubst);
702+
} else
703+
Substitutions.push_back(Subst);
708704
}
709705

710-
auto FnTy = AI.getCallee()->getType().castTo<SILFunctionType>();
711-
assert(FnTy->isPolymorphic());
706+
SILType SubstCalleeType = AI.getSubstCalleeSILType();
707+
708+
SILType NewSubstCalleeType;
712709

713-
auto SFT = FnTy->substGenericArgs(AI.getModule(), Substitutions);
714-
auto NewSubstCalleeType = SILType::getPrimitiveObjectType(SFT);
710+
auto FnTy = AI.getCallee()->getType().castTo<SILFunctionType>();
711+
if (FnTy->isPolymorphic()) {
712+
// Handle polymorphic functions by properly substituting
713+
// their parameter types.
714+
CanSILFunctionType SFT = FnTy->substGenericArgs(
715+
AI.getModule(),
716+
Substitutions);
717+
NewSubstCalleeType = SILType::getPrimitiveObjectType(SFT);
718+
} else {
719+
NewSubstCalleeType =
720+
SubstCalleeType.subst(AI.getModule(),
721+
[&](SubstitutableType *type) -> Type {
722+
if (type == OpenedArchetype)
723+
return ConcreteType;
724+
return type;
725+
},
726+
MakeAbstractConformanceForGenericType());
727+
}
715728

716729
FullApplySite NewAI;
717730
Builder.setCurrentDebugScope(AI.getDebugScope());
718731
Builder.addOpenedArchetypeOperands(AI.getInstruction());
719732

720733
if (auto *TAI = dyn_cast<TryApplyInst>(AI))
721734
NewAI = Builder.createTryApply(AI.getLoc(), AI.getCallee(),
722-
NewSubstCalleeType,
723-
Substitutions, Args,
724-
TAI->getNormalBB(), TAI->getErrorBB());
735+
NewSubstCalleeType,
736+
Substitutions, Args,
737+
TAI->getNormalBB(), TAI->getErrorBB());
725738
else
726739
NewAI = Builder.createApply(AI.getLoc(), AI.getCallee(),
727-
NewSubstCalleeType,
728-
AI.getType(), Substitutions, Args,
729-
cast<ApplyInst>(AI)->isNonThrowing());
740+
NewSubstCalleeType,
741+
AI.getType(), Substitutions, Args,
742+
cast<ApplyInst>(AI)->isNonThrowing());
730743

731744
if (isa<ApplyInst>(NewAI))
732745
replaceInstUsesWith(*AI.getInstruction(), NewAI.getInstruction());
@@ -873,6 +886,21 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
873886
if (WMI->getConformance().isConcrete())
874887
return nullptr;
875888

889+
// Don't specialize Apply instructions that return the Self type.
890+
// Notice that it is sufficient to compare the return type to the
891+
// substituted type because types that depend on the Self type are
892+
// not allowed (for example [Self] is not allowed).
893+
if (AI.getType().getSwiftRValueType() == WMI->getLookupType())
894+
return nullptr;
895+
896+
// We need to handle the Self return type.
897+
// In we find arguments that are not the 'self' argument and if
898+
// they are of the Self type then we abort the optimization.
899+
for (auto Arg : AI.getArgumentsWithoutSelf()) {
900+
if (Arg->getType().getSwiftRValueType() == WMI->getLookupType())
901+
return nullptr;
902+
}
903+
876904
// The lookup type is not an opened existential type,
877905
// thus it cannot be made more concrete.
878906
if (!WMI->getLookupType()->isOpenedExistential())
@@ -888,10 +916,10 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
888916
// Keep around the dependence on the open instruction unless we've
889917
// actually eliminated the use.
890918
auto *NewWMI = Builder.createWitnessMethod(WMI->getLoc(),
891-
ConcreteType,
892-
Conformance, WMI->getMember(),
893-
WMI->getType(),
894-
WMI->isVolatile());
919+
ConcreteType,
920+
Conformance, WMI->getMember(),
921+
WMI->getType(),
922+
WMI->isVolatile());
895923
// Replace only uses of the witness_method in the apply that is going to
896924
// be changed.
897925
MutableArrayRef<Operand> Operands = AI.getInstruction()->getAllOperands();
@@ -929,6 +957,15 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI) {
929957
if (!Self)
930958
return nullptr;
931959

960+
// We need to handle the Self return type.
961+
// In we find arguments that are not the 'self' argument and if
962+
// they are of the Self type then we abort the optimization.
963+
for (auto Arg : AI.getArgumentsWithoutSelf()) {
964+
if (Arg->getType().getSwiftRValueType() ==
965+
AI.getArguments().back()->getType().getSwiftRValueType())
966+
return nullptr;
967+
}
968+
932969
// Obtain the protocol whose which should be used by the conformance.
933970
auto *AFD = dyn_cast<AbstractFunctionDecl>(Callee->getDeclContext());
934971
if (!AFD)

‎test/SILOptimizer/devirtualize_existential.swift

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,61 +8,12 @@ class Foo : Pingable {
88
}
99

1010
// Everything gets devirtualized, inlined, and promoted to the stack.
11-
// CHECK-LABEL: @_T024devirtualize_existential17interesting_stuffyyF
12-
// CHECK-NOT: init_existential_addr
13-
// CHECK-NOT: apply
14-
// CHECK: return
11+
//CHECK: @_T024devirtualize_existential17interesting_stuffyyF
12+
//CHECK-NOT: init_existential_addr
13+
//CHECK-NOT: apply
14+
//CHECK: return
1515
public func interesting_stuff() {
16-
var x: Pingable = Foo()
16+
var x : Pingable = Foo()
1717
x.ping(1)
1818
}
1919

20-
protocol Cloneable {
21-
func clone() -> Self
22-
func maybeClone() -> Self?
23-
}
24-
25-
struct Bar : Cloneable {
26-
@inline(never)
27-
func clone() -> Bar { return self }
28-
29-
@inline(never)
30-
func maybeClone() -> Bar? { return self }
31-
}
32-
33-
// In this example, we don't eliminate the init_existential_addr, because
34-
// of the stack allocated existential value used for the return.
35-
//
36-
// If the combiner was generalized to replace the opened existential type
37-
// with the concrete type in all instructions that use it, instead of just
38-
// special-casing witness_method and apply, we could eliminate the opened
39-
// existential type completely.
40-
//
41-
// However even though IRGen still performs more work at runtime than is
42-
// necessary here, the call is devirtualized.
43-
44-
// CHECK-LABEL: sil @_T024devirtualize_existential22more_interesting_stuffyyF
45-
// CHECK: [[EXISTENTIAL:%.*]] = alloc_stack $Cloneable
46-
// CHECK: [[EXISTENTIAL_ADDR:%.*]] = init_existential_addr [[EXISTENTIAL]]
47-
// CHECK: [[VALUE:%.*]] = struct $Bar ()
48-
// CHECK: [[RESULT_ADDR:%.*]] = unchecked_addr_cast [[EXISTENTIAL_ADDR:%.*]]
49-
// CHECK: [[FN:%.*]] = function_ref @_T024devirtualize_existential3BarV5cloneACyF
50-
// CHECK: [[RETURN:%.*]] = apply [[FN]]([[VALUE]])
51-
// CHECK: store [[RETURN]] to [[RESULT_ADDR]]
52-
53-
// CHECK: [[ENUM:%.*]] = alloc_stack $Optional<Cloneable>
54-
// CHECK: [[ENUM_ADDR:%.*]] = init_enum_data_addr [[ENUM]]
55-
// CHECK: [[EXISTENTIAL_ADDR:%.*]] = init_existential_addr [[ENUM_ADDR]]
56-
// CHECK: [[RESULT_ADDR:%.*]] = unchecked_addr_cast [[EXISTENTIAL_ADDR:%.*]]
57-
// CHECK: [[FN:%.*]] = function_ref @_T024devirtualize_existential3BarV10maybeCloneACSgyF
58-
// CHECK: [[RETURN:%.*]] = apply [[FN]]([[VALUE]])
59-
// CHECK: store [[RETURN]] to [[RESULT_ADDR]]
60-
61-
// CHECK: return
62-
63-
public func more_interesting_stuff() {
64-
var x: Cloneable = Bar()
65-
66-
x.clone()
67-
x.maybeClone()
68-
}

0 commit comments

Comments
 (0)