Skip to content

Commit 9d4b4c7

Browse files
authored
Rewrite SILCombiner::propagateConcreteTypeOfInitExistential. (#17315)
Fixes <rdar://40555427> [SR-7773]: SILCombiner::propagateConcreteTypeOfInitExistential fails to full propagate type substitutions. Fixes <rdar://problem/40923849> SILCombiner::propagateConcreteTypeOfInitExistential crashes on protocol compositions. This rewrite fixes several fundamental bugs in the SILCombiner optimization that propagates concrete types. In particular, the pass needs to handle: - Arguments of callee Self type in non-self position. - Indirect and direct return values of Self type. - Types that indirectly depend on Self within callee function signature. - Protocol composition existentials. - All of the above need to work for protocol extensions as well as witness methods. - For protocol extensions, conformance lookup should be based on the existential's conformance list. Additionally, the optimization should not depend on a SILFunction's DeclContext, which is not serialized. (In fact, we should prevent SIL passes from using DeclContext). Furthermore, the code needs to be expressed in a way that one can reason about correctness and invariants. The root cause of these bugs is that SIL passes are written based on untested assumptions of Swift type system. A SIL pass needs to handle all verifiable SIL input because passes need to be composable. Bail-out logic can be added to simplify the design; however, _the bail-out logic itself cannot make any assumptions about the language or type system_ that aren't clearly and explicitly enforced in the SIL verifier. This is a common mistake and major source of bugs. I created as many unit tests as I reasonably could to prevent this code from regressing. Creating enough unit tests to cover all corner cases that were broken in the original code would be intractable. But the code has been simplified such that many corner cases disappear. This opens up some oportunity for generalizing the optimization and eliminating special cases. However, I want this PR to be limited to fixing correctness issues only. In the long term, it would be preferable to replace this optimization entirely with a much more powerful general type propagation pass.
1 parent 1e5155d commit 9d4b4c7

File tree

8 files changed

+844
-371
lines changed

8 files changed

+844
-371
lines changed

include/swift/SIL/SILBuilder.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,15 @@ class SILBuilderContext {
9292
this->silConv = silConv;
9393
}
9494

95+
void setOpenedArchetypesTracker(SILOpenedArchetypesTracker *Tracker) {
96+
OpenedArchetypesTracker = Tracker;
97+
OpenedArchetypes.setOpenedArchetypesTracker(OpenedArchetypesTracker);
98+
}
99+
100+
SILOpenedArchetypesTracker *getOpenedArchetypesTracker() const {
101+
return OpenedArchetypesTracker;
102+
}
103+
95104
protected:
96105
/// Notify the context of each new instruction after it is inserted in the
97106
/// instruction stream.
@@ -186,12 +195,11 @@ class SILBuilder {
186195
}
187196

188197
void setOpenedArchetypesTracker(SILOpenedArchetypesTracker *Tracker) {
189-
C.OpenedArchetypesTracker = Tracker;
190-
C.OpenedArchetypes.setOpenedArchetypesTracker(C.OpenedArchetypesTracker);
198+
C.setOpenedArchetypesTracker(Tracker);
191199
}
192200

193201
SILOpenedArchetypesTracker *getOpenedArchetypesTracker() const {
194-
return C.OpenedArchetypesTracker;
202+
return C.getOpenedArchetypesTracker();
195203
}
196204

197205
SILOpenedArchetypesState &getOpenedArchetypes() { return C.OpenedArchetypes; }

include/swift/SIL/SILInstruction.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1930,8 +1930,6 @@ class ApplyInstBase<Impl, Base, true>
19301930
assert(hasSelfArgument() && "Must have a self argument");
19311931
assert(getNumArguments() && "Should only be called when Callee has "
19321932
"at least a self parameter.");
1933-
assert(hasSubstitutions() && "Should only be called when Callee has "
1934-
"substitutions.");
19351933
ArrayRef<Operand> ops = this->getArgumentOperands();
19361934
ArrayRef<Operand> opsWithoutSelf = ArrayRef<Operand>(&ops[0],
19371935
ops.size()-1);

include/swift/SILOptimizer/Utils/Existential.h

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,76 @@ SILValue getAddressOfStackInit(AllocStackInst *ASI, SILInstruction *ASIUser,
3131
bool &isCopied);
3232

3333
/// Find the init_existential, which could be used to determine a concrete
34-
/// type of the \p Self.
34+
/// type of the value used by \p openedUse.
3535
/// If the value is copied from another stack location, \p isCopied is set to
3636
/// true.
37-
SILInstruction *findInitExistential(FullApplySite AI, SILValue Self,
37+
///
38+
/// FIXME: replace all uses of this with ConcreteExistentialInfo.
39+
SILInstruction *findInitExistential(Operand &openedUse,
3840
ArchetypeType *&OpenedArchetype,
3941
SILValue &OpenedArchetypeDef,
4042
bool &isCopied);
43+
44+
/// Record conformance and concrete type info derived from an init_existential
45+
/// value that is reopened before it's use. This is useful for finding the
46+
/// concrete type of an apply's self argument. For example, an important pattern
47+
/// for a class existential is:
48+
///
49+
/// %e = init_existential_ref %c : $C : $C, $P & Q
50+
/// %o = open_existential_ref %e : $P & Q to $@opened("PQ") P & Q
51+
/// %r = apply %f<@opened("PQ") P & Q>(%o)
52+
/// : $@convention(method) <τ_0_0 where τ_0_0 : P, τ_0_0 : Q>
53+
/// (@guaranteed τ_0_0) -> @owned τ_0_0
54+
struct ConcreteExistentialInfo {
55+
// The opened type passed as self. `$@opened("PQ")` above.
56+
// This is also the replacement type of the method's Self type.
57+
ArchetypeType *OpenedArchetype = nullptr;
58+
// The definition of the OpenedArchetype.
59+
SILValue OpenedArchetypeDef;
60+
// True if the openedValue is copied from another stack location
61+
bool isCopied;
62+
// The init_existential instruction that produces the opened existential.
63+
SILInstruction *InitExistential = nullptr;
64+
// The existential type of the self argument before it is opened,
65+
// produced by an init_existential.
66+
CanType ExistentialType;
67+
// The conformances required by the init_existential. `$C : $P & $Q` above.
68+
ArrayRef<ProtocolConformanceRef> ExistentialConformances;
69+
// The concrete type of self from the init_existential. `$C` above.
70+
CanType ConcreteType;
71+
// When ConcreteType is itself an opened existential, record the type
72+
// definition. May be nullptr for a valid AppliedConcreteType.
73+
SingleValueInstruction *ConcreteTypeDef = nullptr;
74+
// The Substitution map derived from the init_existential.
75+
// This maps a single generic parameter to the replacement ConcreteType
76+
// and includes the full list of existential conformances.
77+
// signature: <P & Q>, replacement: $C : conformances: [$P, $Q]
78+
SubstitutionMap ExistentialSubs;
79+
// The value of concrete type used to initialize the existential. `%c` above.
80+
SILValue ConcreteValue;
81+
82+
// Search for a recognized pattern in which the given value is an opened
83+
// existential that was previously initialized to a concrete type.
84+
// Constructs a valid ConcreteExistentialInfo object if successfull.
85+
ConcreteExistentialInfo(Operand &openedUse);
86+
87+
ConcreteExistentialInfo(ConcreteExistentialInfo &) = delete;
88+
89+
bool isValid() const {
90+
return OpenedArchetype && OpenedArchetypeDef && InitExistential
91+
&& ConcreteType && !ExistentialSubs.empty() && ConcreteValue;
92+
}
93+
94+
// Do a conformance lookup on ConcreteType with the given requirement, P. If P
95+
// is satisfiable based on the existential's conformance, return the new
96+
// conformance on P. Otherwise return None.
97+
Optional<ProtocolConformanceRef>
98+
lookupExistentialConformance(ProtocolDecl *P) const {
99+
CanType selfTy = P->getSelfInterfaceType()->getCanonicalType();
100+
return ExistentialSubs.lookupConformance(selfTy, P);
101+
}
102+
};
103+
41104
} // end namespace swift
42105

43106
#endif

lib/SILOptimizer/SILCombiner/SILCombiner.h

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "swift/SIL/SILValue.h"
2727
#include "swift/SIL/SILVisitor.h"
2828
#include "swift/SILOptimizer/Utils/CastOptimizer.h"
29+
#include "swift/SILOptimizer/Utils/Existential.h"
2930
#include "swift/SILOptimizer/Utils/Local.h"
3031
#include "llvm/ADT/DenseMap.h"
3132
#include "llvm/ADT/SmallVector.h"
@@ -279,24 +280,16 @@ class SILCombiner :
279280
StringRef FInverseName, StringRef FName);
280281

281282
private:
282-
SILInstruction * createApplyWithConcreteType(FullApplySite AI,
283-
SILValue NewSelf,
284-
SILValue Self,
285-
CanType ConcreteType,
286-
SILValue ConcreteTypeDef,
287-
ProtocolConformanceRef Conformance,
288-
ArchetypeType *OpenedArchetype);
289-
290283
FullApplySite rewriteApplyCallee(FullApplySite apply, SILValue callee);
291284

292-
SILInstruction *
293-
propagateConcreteTypeOfInitExistential(FullApplySite AI,
294-
ProtocolDecl *Protocol,
295-
llvm::function_ref<void(CanType, ProtocolConformanceRef)> Propagate);
285+
SILInstruction *createApplyWithConcreteType(FullApplySite Apply,
286+
const ConcreteExistentialInfo &CEI,
287+
SILBuilderContext &BuilderCtx);
296288

297-
SILInstruction *propagateConcreteTypeOfInitExistential(FullApplySite AI,
298-
WitnessMethodInst *WMI);
299-
SILInstruction *propagateConcreteTypeOfInitExistential(FullApplySite AI);
289+
SILInstruction *
290+
propagateConcreteTypeOfInitExistential(FullApplySite Apply,
291+
WitnessMethodInst *WMI);
292+
SILInstruction *propagateConcreteTypeOfInitExistential(FullApplySite Apply);
300293

301294
/// Perform one SILCombine iteration.
302295
bool doOneIteration(SILFunction &F, unsigned Iteration);

0 commit comments

Comments
 (0)