Skip to content

Commit 3d9a79e

Browse files
authored
Merge pull request #64777 from slavapestov/sil-optimizer-variadic-generics
SIL optimizer fixes for variadic generics
2 parents 1957297 + 7ad2365 commit 3d9a79e

File tree

17 files changed

+158
-100
lines changed

17 files changed

+158
-100
lines changed

SwiftCompilerSources/Sources/SIL/Argument.swift

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,26 @@ public enum ArgumentConvention {
122122
/// guarantees its validity for the entirety of the call.
123123
case directGuaranteed
124124

125+
/// This argument is a value pack of mutable references to storage,
126+
/// which the function is being given exclusive access to. The elements
127+
/// must be passed indirectly.
128+
case packInout
129+
130+
/// This argument is a value pack, and ownership of the elements is being
131+
/// transferred into this function. Whether the elements are passed
132+
/// indirectly is recorded in the pack type.
133+
case packOwned
134+
135+
/// This argument is a value pack, and ownership of the elements is not
136+
/// being transferred into this function. Whether the elements are passed
137+
/// indirectly is recorded in the pack type.
138+
case packGuaranteed
139+
125140
public var isIndirect: Bool {
126141
switch self {
127142
case .indirectIn, .indirectInGuaranteed,
128-
.indirectInout, .indirectInoutAliasable, .indirectOut:
143+
.indirectInout, .indirectInoutAliasable, .indirectOut,
144+
.packInout, .packOwned, .packGuaranteed:
129145
return true
130146
case .directOwned, .directUnowned, .directGuaranteed:
131147
return false
@@ -134,20 +150,23 @@ public enum ArgumentConvention {
134150

135151
public var isIndirectIn: Bool {
136152
switch self {
137-
case .indirectIn, .indirectInGuaranteed:
153+
case .indirectIn, .indirectInGuaranteed,
154+
.packOwned, .packGuaranteed:
138155
return true
139156
case .directOwned, .directUnowned, .directGuaranteed,
140-
.indirectInout, .indirectInoutAliasable, .indirectOut:
157+
.indirectInout, .indirectInoutAliasable, .indirectOut,
158+
.packInout:
141159
return false
142160
}
143161
}
144162

145163
public var isGuaranteed: Bool {
146164
switch self {
147-
case .indirectInGuaranteed, .directGuaranteed:
165+
case .indirectInGuaranteed, .directGuaranteed, .packGuaranteed:
148166
return true
149167
case .indirectIn, .directOwned, .directUnowned,
150-
.indirectInout, .indirectInoutAliasable, .indirectOut:
168+
.indirectInout, .indirectInoutAliasable, .indirectOut,
169+
.packInout, .packOwned:
151170
return false
152171
}
153172
}
@@ -157,7 +176,10 @@ public enum ArgumentConvention {
157176
case .indirectIn,
158177
.indirectOut,
159178
.indirectInGuaranteed,
160-
.indirectInout:
179+
.indirectInout,
180+
.packInout,
181+
.packOwned,
182+
.packGuaranteed:
161183
return true
162184

163185
case .indirectInoutAliasable,
@@ -171,15 +193,18 @@ public enum ArgumentConvention {
171193
public var isInout: Bool {
172194
switch self {
173195
case .indirectInout,
174-
.indirectInoutAliasable:
196+
.indirectInoutAliasable,
197+
.packInout:
175198
return true
176199

177200
case .indirectIn,
178201
.indirectOut,
179202
.indirectInGuaranteed,
180203
.directUnowned,
181204
.directGuaranteed,
182-
.directOwned:
205+
.directOwned,
206+
.packOwned,
207+
.packGuaranteed:
183208
return false
184209
}
185210
}
@@ -204,6 +229,9 @@ extension BridgedArgumentConvention {
204229
case .Direct_Owned: return .directOwned
205230
case .Direct_Unowned: return .directUnowned
206231
case .Direct_Guaranteed: return .directGuaranteed
232+
case .Pack_Inout: return .packInout
233+
case .Pack_Owned: return .packOwned
234+
case .Pack_Guaranteed: return .packGuaranteed
207235
default:
208236
fatalError("unsupported argument convention")
209237
}

SwiftCompilerSources/Sources/SIL/Effects.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -541,12 +541,12 @@ public struct SideEffects : CustomStringConvertible, NoReflectionChildren {
541541
result.ownership = SideEffects.Ownership()
542542
}
543543
switch convention {
544-
case .indirectIn:
544+
case .indirectIn, .packOwned:
545545
result.memory.write = false
546-
case .indirectInGuaranteed:
546+
case .indirectInGuaranteed, .packGuaranteed:
547547
result.memory.write = false
548548
result.ownership.destroy = false
549-
case .indirectOut:
549+
case .indirectOut, .packInout:
550550
result.memory.read = false
551551
result.ownership.copy = false
552552
result.ownership.destroy = false

include/swift/AST/Types.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class Identifier;
7171
class InOutType;
7272
class OpaqueTypeDecl;
7373
class OpenedArchetypeType;
74+
class PackExpansionType;
7475
class PackType;
7576
enum class ParamSpecifier : uint8_t;
7677
class PlaceholderTypeRepr;
@@ -2414,6 +2415,12 @@ class TupleType final : public TypeBase, public llvm::FoldingSetNode,
24142415

24152416
unsigned getNumElements() const { return Bits.TupleType.Count; }
24162417

2418+
/// Returns the number of non-PackExpansionType elements. This is the
2419+
/// minimum length of the tuple after substitution; a tuple with
2420+
/// zero or one scalar elements is unwrapped if it would otherwise be
2421+
/// a one-element tuple after substitution.
2422+
unsigned getNumScalarElements() const;
2423+
24172424
/// getElements - Return the elements of this tuple.
24182425
ArrayRef<TupleTypeElt> getElements() const {
24192426
return {getTrailingObjects<TupleTypeElt>(), getNumElements()};
@@ -6838,6 +6845,8 @@ class PackType final : public TypeBase, public llvm::FoldingSetNode,
68386845

68396846
bool containsPackExpansionType() const;
68406847

6848+
PackExpansionType *unwrapSingletonPackExpansion() const;
6849+
68416850
CanTypeWrapper<PackType> getReducedShape();
68426851

68436852
public:
@@ -6876,6 +6885,8 @@ BEGIN_CAN_TYPE_WRAPPER(PackType, Type)
68766885
CanTypeArrayRef getElementTypes() const {
68776886
return CanTypeArrayRef(getPointer()->getElementTypes());
68786887
}
6888+
6889+
CanTypeWrapper<PackExpansionType> unwrapSingletonPackExpansion() const;
68796890
END_CAN_TYPE_WRAPPER(PackType, Type)
68806891

68816892
inline CanPackType CanTupleType::getInducedPackType() const {
@@ -6964,6 +6975,13 @@ BEGIN_CAN_TYPE_WRAPPER(PackExpansionType, Type)
69646975
}
69656976
END_CAN_TYPE_WRAPPER(PackExpansionType, Type)
69666977

6978+
6979+
inline CanTypeWrapper<PackExpansionType>
6980+
CanPackType::unwrapSingletonPackExpansion() const {
6981+
return CanPackExpansionType(
6982+
getPointer()->unwrapSingletonPackExpansion());
6983+
}
6984+
69676985
/// getASTContext - Return the ASTContext that this type belongs to.
69686986
inline ASTContext &TypeBase::getASTContext() const {
69696987
// If this type is canonical, it has the ASTContext in it.

include/swift/SIL/SILCloner.h

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -279,34 +279,22 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
279279
if (type->getNumElements() == 0) return false;
280280

281281
// Do a first pass over the tuple elements to check out the
282-
// non-expansions. If there's more than one of them, or any of them
283-
// is labeled, we definitely stay a tuple and don't need to substitute
284-
// any of the expansions.
285-
unsigned numScalarElements = 0;
286-
for (auto index : indices(type->getElements())) {
287-
auto eltType = type.getElementType(index);
288-
// Ignore pack expansions in this pass.
289-
if (isa<PackExpansionType>(eltType)) continue;
290-
291-
// If there's a labeled scalar element, we'll stay a tuple.
292-
if (type->getElement(index).hasName()) return false;
293-
294-
// If there are multiple scalar elements, we'll stay a tuple.
295-
if (++numScalarElements > 1) return false;
296-
}
297-
298-
assert(numScalarElements <= 1);
299-
300-
// We must have expansions if we got here: if all the elements were
301-
// scalar, and none of them were labelled, and there wasn't more than
302-
// one of them, and there was at least one of them, then somehow
303-
// we had a tuple with a single unlabeled element.
282+
// non-expansions. If there's more than one of them we definitely
283+
// stay a tuple and don't need to substitute any of the expansions.
284+
unsigned numScalarElements = type->getNumScalarElements();
285+
if (numScalarElements > 1)
286+
return false;
304287

305288
// Okay, we need to substitute the count types for the expansions.
306289
for (auto index : indices(type->getElements())) {
307290
// Ignore non-expansions because we've already counted them.
308291
auto expansion = dyn_cast<PackExpansionType>(type.getElementType(index));
309-
if (!expansion) continue;
292+
if (!expansion) {
293+
// If we have a non-expansion with a label, we stay a tuple.
294+
if (type->getElement(index).hasName())
295+
return false;
296+
continue;
297+
}
310298

311299
// Substitute the shape class of the expansion.
312300
auto newShapeClass = getOpASTType(expansion.getCountType());

lib/AST/ParameterPack.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,16 @@ CanType PackExpansionType::getReducedShape() {
102102
return CanType(PackExpansionType::get(reducedShape, reducedShape));
103103
}
104104

105+
unsigned TupleType::getNumScalarElements() const {
106+
unsigned n = 0;
107+
for (auto elt : getElements()) {
108+
if (!elt.getType()->is<PackExpansionType>())
109+
++n;
110+
}
111+
112+
return n;
113+
}
114+
105115
bool TupleType::containsPackExpansionType() const {
106116
for (auto elt : getElements()) {
107117
if (elt.getType()->is<PackExpansionType>())
@@ -290,6 +300,18 @@ CanPackType CanPackType::getSingletonPackExpansion(CanType param) {
290300
return CanPackType(PackType::getSingletonPackExpansion(param));
291301
}
292302

303+
PackExpansionType *PackType::unwrapSingletonPackExpansion() const {
304+
if (getNumElements() == 1) {
305+
if (auto expansion = getElementTypes()[0]->getAs<PackExpansionType>()) {
306+
auto pattern = expansion->getPatternType();
307+
if (pattern->isParameterPack() || pattern->is<PackArchetypeType>())
308+
return expansion;
309+
}
310+
}
311+
312+
return nullptr;
313+
}
314+
293315
bool SILPackType::containsPackExpansionType() const {
294316
for (auto type : getElementTypes()) {
295317
if (isa<PackExpansionType>(type))

lib/IRGen/Fulfillment.cpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,14 +182,12 @@ bool FulfillmentMap::searchTypeMetadata(IRGenModule &IGM, CanType type,
182182
static CanType getSingletonPackExpansionParameter(CanPackType packType,
183183
const FulfillmentMap::InterestingKeysCallback &keys,
184184
Optional<unsigned> &packExpansionComponent) {
185-
if (packType->getNumElements() != 1)
186-
return CanType();
187-
auto expansion = dyn_cast<PackExpansionType>(packType.getElementType(0));
188-
if (!expansion || !keys.isInterestingPackExpansion(expansion))
189-
return CanType();
190-
191-
packExpansionComponent = 0;
192-
return expansion.getPatternType();
185+
if (auto expansion = packType.unwrapSingletonPackExpansion()) {
186+
if (keys.isInterestingPackExpansion(expansion)) {
187+
packExpansionComponent = 0;
188+
return expansion.getPatternType();
189+
}
190+
}
193191
}
194192

195193
bool FulfillmentMap::searchTypeMetadataPack(IRGenModule &IGM,
@@ -405,10 +403,7 @@ bool FulfillmentMap::searchShapeRequirement(IRGenModule &IGM, CanType argType,
405403
// there aren't expansions over pack parameters with different shapes,
406404
// we should always be able to turn this into the equation
407405
// `ax + b = <fulfilled count>` and then solve that.
408-
if (packType->getNumElements() != 1)
409-
return false;
410-
auto expansion =
411-
dyn_cast<PackExpansionType>(packType.getElementType(0));
406+
auto expansion = packType.unwrapSingletonPackExpansion();
412407
if (!expansion)
413408
return false;
414409

0 commit comments

Comments
 (0)