Skip to content

Commit 77b6e7c

Browse files
authored
4.2 SILCombiner::propagateConcreteTypeOfInitExistential fixes. (#17554)
* Reorganizing the code to find init_existential; Move them to Existential.cpp/h in order for other passes such as ExistentialSpecializer to use it apart from SILCombiner * [NFC] Add SILBuilderContext. In an upcoming bug fix, I want to pass SILBuilderContext to a utility. I could continue reusing SILBuilder, even though the utility must set its own insertion point and debug location. However, this is terrible practice that I don't want to perpetuate. The lifetime of a SILBuilder should correspond to a single insertion point and debug location. That's the only sane way to preserve debug information in SIL passes. There are various pieces of contextual state that we've been adding to the SILBuilder. Those have made it impossible to use SILBuilder correctly. I'm pulling the context out, so clients can begin using better practices. In the future, we can make SILBuilderContext polymorphic, so passes can extend it easily with arbitrary callbacks. We can also make it self-contained so we don't need to pass around pointers to an InsertedInst list anymore. (cherry picked from commit df94dff) * 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. (cherry picked from commit 9d4b4c7) * Merge rewrite SILCombiner::propagateConcreteTypeOfInitExistential onto 4.2.
1 parent 943290e commit 77b6e7c

File tree

10 files changed

+1276
-620
lines changed

10 files changed

+1276
-620
lines changed

include/swift/SIL/SILBuilder.h

Lines changed: 185 additions & 114 deletions
Large diffs are not rendered by default.

include/swift/SIL/SILInstruction.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,8 +1955,6 @@ class ApplyInstBase<Impl, Base, true>
19551955
assert(hasSelfArgument() && "Must have a self argument");
19561956
assert(getNumArguments() && "Should only be called when Callee has "
19571957
"at least a self parameter.");
1958-
assert(hasSubstitutions() && "Should only be called when Callee has "
1959-
"substitutions.");
19601958
ArrayRef<Operand> ops = this->getArgumentOperands();
19611959
ArrayRef<Operand> opsWithoutSelf = ArrayRef<Operand>(&ops[0],
19621960
ops.size()-1);
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
//===--- Existential.h - Existential related Analyses. -------*- C++ //-*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef SWIFT_SILOPTIMIZER_UTILS_EXISTENTIAL_H
14+
#define SWIFT_SILOPTIMIZER_UTILS_EXISTENTIAL_H
15+
16+
#include "swift/AST/SubstitutionMap.h"
17+
#include "swift/SIL/SILInstruction.h"
18+
19+
namespace swift {
20+
21+
/// Find InitExistential from global_addr and copy_addr.
22+
SILValue findInitExistentialFromGlobalAddr(GlobalAddrInst *GAI,
23+
CopyAddrInst *CAI);
24+
25+
/// Returns the address of an object with which the stack location \p ASI is
26+
/// initialized. This is either a init_existential_addr or the destination of a
27+
/// copy_addr. Returns a null value if the address does not dominate the
28+
/// alloc_stack user \p ASIUser.
29+
/// If the value is copied from another stack location, \p isCopied is set to
30+
/// true.
31+
SILValue getAddressOfStackInit(AllocStackInst *ASI, SILInstruction *ASIUser,
32+
bool &isCopied);
33+
34+
/// Find the init_existential, which could be used to determine a concrete
35+
/// type of the value used by \p openedUse.
36+
/// If the value is copied from another stack location, \p isCopied is set to
37+
/// true.
38+
///
39+
/// FIXME: replace all uses of this with ConcreteExistentialInfo.
40+
SILInstruction *findInitExistential(Operand &openedUse,
41+
ArchetypeType *&OpenedArchetype,
42+
SILValue &OpenedArchetypeDef,
43+
bool &isCopied);
44+
45+
/// Record conformance and concrete type info derived from an init_existential
46+
/// value that is reopened before it's use. This is useful for finding the
47+
/// concrete type of an apply's self argument. For example, an important pattern
48+
/// for a class existential is:
49+
///
50+
/// %e = init_existential_ref %c : $C : $C, $P & Q
51+
/// %o = open_existential_ref %e : $P & Q to $@opened("PQ") P & Q
52+
/// %r = apply %f<@opened("PQ") P & Q>(%o)
53+
/// : $@convention(method) <τ_0_0 where τ_0_0 : P, τ_0_0 : Q>
54+
/// (@guaranteed τ_0_0) -> @owned τ_0_0
55+
struct ConcreteExistentialInfo {
56+
// The opened type passed as self. `$@opened("PQ")` above.
57+
// This is also the replacement type of the method's Self type.
58+
ArchetypeType *OpenedArchetype = nullptr;
59+
// The definition of the OpenedArchetype.
60+
SILValue OpenedArchetypeDef;
61+
// True if the openedValue is copied from another stack location
62+
bool isCopied;
63+
// The init_existential instruction that produces the opened existential.
64+
SILInstruction *InitExistential = nullptr;
65+
// The existential type of the self argument before it is opened,
66+
// produced by an init_existential.
67+
CanType ExistentialType;
68+
// The conformances required by the init_existential. `$C : $P & $Q` above.
69+
ArrayRef<ProtocolConformanceRef> ExistentialConformances;
70+
// The concrete type of self from the init_existential. `$C` above.
71+
CanType ConcreteType;
72+
// When ConcreteType is itself an opened existential, record the type
73+
// definition. May be nullptr for a valid AppliedConcreteType.
74+
SingleValueInstruction *ConcreteTypeDef = nullptr;
75+
// The Substitution map derived from the init_existential.
76+
// This maps a single generic parameter to the replacement ConcreteType
77+
// and includes the full list of existential conformances.
78+
// signature: <P & Q>, replacement: $C : conformances: [$P, $Q]
79+
SubstitutionMap ExistentialSubs;
80+
// The value of concrete type used to initialize the existential. `%c` above.
81+
SILValue ConcreteValue;
82+
83+
// Search for a recognized pattern in which the given value is an opened
84+
// existential that was previously initialized to a concrete type.
85+
// Constructs a valid ConcreteExistentialInfo object if successfull.
86+
ConcreteExistentialInfo(Operand &openedUse);
87+
88+
ConcreteExistentialInfo(ConcreteExistentialInfo &) = delete;
89+
90+
bool isValid() const {
91+
return OpenedArchetype && OpenedArchetypeDef && InitExistential
92+
&& ConcreteType && !ExistentialSubs.empty() && ConcreteValue;
93+
}
94+
95+
// Do a conformance lookup on ConcreteType with the given requirement, P. If P
96+
// is satisfiable based on the existential's conformance, return the new
97+
// conformance on P. Otherwise return None.
98+
Optional<ProtocolConformanceRef>
99+
lookupExistentialConformance(ProtocolDecl *P) const {
100+
CanType selfTy = P->getSelfInterfaceType()->getCanonicalType();
101+
return ExistentialSubs.lookupConformance(selfTy, P);
102+
}
103+
};
104+
105+
} // end namespace swift
106+
107+
#endif

lib/SIL/SILBuilder.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ using namespace swift;
2323

2424
SILBuilder::SILBuilder(SILGlobalVariable *GlobVar,
2525
SmallVectorImpl<SILInstruction *> *InsertedInstrs)
26-
: F(nullptr), Mod(GlobVar->getModule()), InsertedInstrs(InsertedInstrs) {
26+
: TempContext(GlobVar->getModule(), InsertedInstrs), C(TempContext),
27+
F(nullptr) {
2728
setInsertionPoint(&GlobVar->StaticInitializerBlock);
2829
}
2930

@@ -100,7 +101,8 @@ SILBuilder::tryCreateUncheckedRefCast(SILLocation Loc, SILValue Op,
100101
return nullptr;
101102

102103
return insert(UncheckedRefCastInst::create(getSILDebugLocation(Loc), Op,
103-
ResultTy, getFunction(), OpenedArchetypes));
104+
ResultTy, getFunction(),
105+
C.OpenedArchetypes));
104106
}
105107

106108
ClassifyBridgeObjectInst *
@@ -120,15 +122,15 @@ SILBuilder::createUncheckedBitCast(SILLocation Loc, SILValue Op, SILType Ty) {
120122
assert(Ty.isLoadableOrOpaque(getModule()));
121123
if (Ty.isTrivial(getModule()))
122124
return insert(UncheckedTrivialBitCastInst::create(
123-
getSILDebugLocation(Loc), Op, Ty, getFunction(), OpenedArchetypes));
125+
getSILDebugLocation(Loc), Op, Ty, getFunction(), C.OpenedArchetypes));
124126

125127
if (auto refCast = tryCreateUncheckedRefCast(Loc, Op, Ty))
126128
return refCast;
127129

128130
// The destination type is nontrivial, and may be smaller than the source
129131
// type, so RC identity cannot be assumed.
130-
return insert(UncheckedBitwiseCastInst::create(getSILDebugLocation(Loc), Op,
131-
Ty, getFunction(), OpenedArchetypes));
132+
return insert(UncheckedBitwiseCastInst::create(
133+
getSILDebugLocation(Loc), Op, Ty, getFunction(), C.OpenedArchetypes));
132134
}
133135

134136
BranchInst *SILBuilder::createBranch(SILLocation Loc,
@@ -420,7 +422,7 @@ SILValue SILBuilder::emitObjCToThickMetatype(SILLocation Loc, SILValue Op,
420422
void SILBuilder::addOpenedArchetypeOperands(SILInstruction *I) {
421423
// The list of archetypes from the previous instruction needs
422424
// to be replaced, because it may reference a removed instruction.
423-
OpenedArchetypes.addOpenedArchetypeOperands(I->getTypeDependentOperands());
425+
C.OpenedArchetypes.addOpenedArchetypeOperands(I->getTypeDependentOperands());
424426
if (I && I->getNumTypeDependentOperands() > 0)
425427
return;
426428

@@ -447,18 +449,19 @@ void SILBuilder::addOpenedArchetypeOperands(SILInstruction *I) {
447449
I = SVI;
448450
continue;
449451
}
450-
auto Def = OpenedArchetypes.getOpenedArchetypeDef(Archetype);
452+
auto Def = C.OpenedArchetypes.getOpenedArchetypeDef(Archetype);
451453
// Return if it is a known open archetype.
452454
if (Def)
453455
return;
454456
// Otherwise register it and return.
455-
if (OpenedArchetypesTracker)
456-
OpenedArchetypesTracker->addOpenedArchetypeDef(Archetype, SVI);
457+
if (C.OpenedArchetypesTracker)
458+
C.OpenedArchetypesTracker->addOpenedArchetypeDef(Archetype, SVI);
457459
return;
458460
}
459461

460462
if (I && I->getNumTypeDependentOperands() > 0) {
461-
OpenedArchetypes.addOpenedArchetypeOperands(I->getTypeDependentOperands());
463+
C.OpenedArchetypes.addOpenedArchetypeOperands(
464+
I->getTypeDependentOperands());
462465
}
463466
}
464467

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)