Skip to content

Commit 239777a

Browse files
committed
Fix parameter binding for tuples containing pack expansions
More missing infrastructure. In this case, it's really *existing* missing infrastructure, though; we should have been imploding tuples this way all along, given that we're doing it in the first place. I don't like that we're doing all these extra tuple copies. I'm not sure yet if they're just coming out of SILGen and eliminated immediately after in practice; maybe so. Still, it should be obvious that they're unnecessary.
1 parent 3faee07 commit 239777a

15 files changed

+602
-73
lines changed

include/swift/SIL/SILType.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,14 @@ class SILType {
531531
return SILType(castTo<TupleType>().getElementType(index), getCategory());
532532
}
533533

534+
/// Given that this is a pack expansion type, return the lowered type
535+
/// of the pattern type. The result will have the same value category
536+
/// as the base type.
537+
SILType getPackExpansionPatternType() const {
538+
return SILType(castTo<PackExpansionType>().getPatternType(),
539+
getCategory());
540+
}
541+
534542
/// Return the immediate superclass type of this type, or null if
535543
/// it's the most-derived type.
536544
SILType getSuperclass() const {

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1705,7 +1705,7 @@ class DestructureInputs {
17051705
packElts.push_back(eltTy.getASTType());
17061706
});
17071707

1708-
bool indirect = origType.arePackElementsPassedIndirectly(TC);
1708+
bool indirect = origEltType.arePackElementsPassedIndirectly(TC);
17091709
SILPackType::ExtInfo extInfo(/*address*/ indirect);
17101710
auto packTy = SILPackType::get(TC.Context, extInfo, packElts);
17111711

lib/SILGen/Cleanup.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,3 +420,27 @@ ManagedValue CleanupCloner::clone(SILValue value) const {
420420
}
421421
return SGF.emitManagedRValueWithCleanup(value);
422422
}
423+
424+
ManagedValue
425+
CleanupCloner::cloneForTuplePackExpansionComponent(SILValue tupleAddr,
426+
CanPackType inducedPackType,
427+
unsigned componentIndex) const {
428+
if (isLValue) {
429+
return ManagedValue::forLValue(tupleAddr);
430+
}
431+
432+
if (!hasCleanup) {
433+
return ManagedValue::forUnmanaged(tupleAddr);
434+
}
435+
436+
assert(!writebackBuffer.has_value());
437+
auto expansionTy = tupleAddr->getType().getTupleElementType(componentIndex);
438+
if (expansionTy.getPackExpansionPatternType().isTrivial(SGF.F))
439+
return ManagedValue::forUnmanaged(tupleAddr);
440+
441+
auto cleanup =
442+
SGF.enterPartialDestroyRemainingTupleCleanup(tupleAddr, inducedPackType,
443+
componentIndex,
444+
/*start at */ SILValue());
445+
return ManagedValue(tupleAddr, cleanup);
446+
}

lib/SILGen/Cleanup.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,10 @@ class CleanupCloner {
338338

339339
ManagedValue clone(SILValue value) const;
340340

341+
ManagedValue cloneForTuplePackExpansionComponent(SILValue value,
342+
CanPackType inducedPackType,
343+
unsigned componentIndex) const;
344+
341345
static void
342346
getClonersForRValue(SILGenFunction &SGF, const RValue &rvalue,
343347
SmallVectorImpl<CleanupCloner> &resultingCloners);

lib/SILGen/Initialization.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,13 +330,16 @@ class TemporaryInitialization : public SingleBufferInitialization {
330330
/// An initialization which accumulates several other initializations
331331
/// into a tuple.
332332
class TupleInitialization : public Initialization {
333+
CanTupleType FormalTupleType;
334+
333335
public:
334336
/// The sub-Initializations aggregated by this tuple initialization.
335337
/// The TupleInitialization object takes ownership of Initializations pushed
336338
/// here.
337339
SmallVector<InitializationPtr, 4> SubInitializations;
338-
339-
TupleInitialization() {}
340+
341+
TupleInitialization(CanTupleType formalTupleType)
342+
: FormalTupleType(formalTupleType) {}
340343

341344
bool canSplitIntoTupleElements() const override {
342345
return true;

lib/SILGen/ResultPlan.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -506,9 +506,7 @@ class PackTransformResultPlan final : public ResultPlan {
506506

507507
// Loop over the pack, initializing each value with the appropriate
508508
// element.
509-
SGF.emitDynamicPackLoop(loc, FormalPackType, ComponentIndex,
510-
/*limit*/SILValue(), openedEnv,
511-
/*reverse*/false,
509+
SGF.emitDynamicPackLoop(loc, FormalPackType, ComponentIndex, openedEnv,
512510
[&](SILValue indexWithinComponent,
513511
SILValue expansionIndex,
514512
SILValue packIndex) {
@@ -1203,9 +1201,7 @@ ResultPlanBuilder::buildPackExpansionIntoPack(SILValue packAddr,
12031201
// If the expansion addresses can just be forwarded into the pack,
12041202
// we can emit a dynamic loop to do that now.
12051203
if (init->canPerformInPlacePackInitialization(openedEnv, eltTy)) {
1206-
SGF.emitDynamicPackLoop(loc, formalPackType, componentIndex,
1207-
/*limit*/ SILValue(), openedEnv,
1208-
/*reverse*/ false,
1204+
SGF.emitDynamicPackLoop(loc, formalPackType, componentIndex, openedEnv,
12091205
[&](SILValue indexWithinComponent,
12101206
SILValue expansionPackIndex,
12111207
SILValue packIndex) {
@@ -1226,8 +1222,7 @@ ResultPlanBuilder::buildPackExpansionIntoPack(SILValue packAddr,
12261222
auto tupleAddr = SGF.emitTemporaryAllocation(loc,
12271223
SILType::getPrimitiveObjectType(tupleTy));
12281224

1229-
SGF.emitDynamicPackLoop(loc, formalPackType, componentIndex,
1230-
/*limit*/ SILValue(), openedEnv, /*reverse*/ false,
1225+
SGF.emitDynamicPackLoop(loc, formalPackType, componentIndex, openedEnv,
12311226
[&](SILValue indexWithinComponent,
12321227
SILValue expansionPackIndex,
12331228
SILValue packIndex) {

lib/SILGen/SILGenApply.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3809,8 +3809,7 @@ class ArgEmitter {
38093809

38103810
auto openedElementEnv = expansionExpr->getGenericEnvironment();
38113811
SGF.emitDynamicPackLoop(expansionExpr, formalPackType,
3812-
packComponentIndex, /*limit*/ nullptr,
3813-
openedElementEnv, /*reverse*/false,
3812+
packComponentIndex, openedElementEnv,
38143813
[&](SILValue indexWithinComponent,
38153814
SILValue packExpansionIndex,
38163815
SILValue packIndex) {

lib/SILGen/SILGenConstructor.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1062,7 +1062,8 @@ emitMemberInit(SILGenFunction &SGF, VarDecl *selfDecl, Pattern *pattern) {
10621062
cast<ParenPattern>(pattern)->getSubPattern());
10631063

10641064
case PatternKind::Tuple: {
1065-
TupleInitialization *init = new TupleInitialization();
1065+
TupleInitialization *init = new TupleInitialization(
1066+
cast<TupleType>(pattern->getType()->getCanonicalType()));
10661067
auto tuple = cast<TuplePattern>(pattern);
10671068
for (auto &elt : tuple->getElements()) {
10681069
init->SubInitializations.push_back(

lib/SILGen/SILGenDecl.cpp

Lines changed: 120 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,96 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
4949
void Initialization::_anchor() {}
5050
void SILDebuggerClient::anchor() {}
5151

52+
static void copyOrInitPackExpansionInto(SILGenFunction &SGF,
53+
SILLocation loc,
54+
SILValue tupleAddr,
55+
CanPackType formalPackType,
56+
unsigned componentIndex,
57+
CleanupHandle componentCleanup,
58+
Initialization *expansionInit,
59+
bool isInit) {
60+
auto expansionTy = tupleAddr->getType().getTupleElementType(componentIndex);
61+
assert(expansionTy.is<PackExpansionType>());
62+
63+
auto opening = SGF.createOpenedElementValueEnvironment(expansionTy);
64+
auto openedEnv = opening.first;
65+
auto eltTy = opening.second;
66+
67+
assert(expansionInit);
68+
assert(expansionInit->canPerformPackExpansionInitialization());
69+
70+
// Exit the component-wide cleanup for the expansion component.
71+
if (componentCleanup.isValid())
72+
SGF.Cleanups.forwardCleanup(componentCleanup);
73+
74+
SGF.emitDynamicPackLoop(loc, formalPackType, componentIndex, openedEnv,
75+
[&](SILValue indexWithinComponent,
76+
SILValue packExpansionIndex,
77+
SILValue packIndex) {
78+
expansionInit->performPackExpansionInitialization(SGF, loc,
79+
indexWithinComponent,
80+
[&](Initialization *eltInit) {
81+
// Project the current tuple element.
82+
auto eltAddr =
83+
SGF.B.createTuplePackElementAddr(loc, packIndex, tupleAddr, eltTy);
84+
85+
SILValue elt = eltAddr;
86+
if (!eltTy.isAddressOnly(SGF.F)) {
87+
elt = SGF.B.emitLoadValueOperation(loc, elt,
88+
LoadOwnershipQualifier::Take);
89+
}
90+
91+
// Enter a cleanup for the current element, which we need to consume
92+
// on this iteration of the loop, and the remaining elements in the
93+
// expansion component, which we need to destroy if we throw from
94+
// the initialization.
95+
CleanupHandle eltCleanup = CleanupHandle::invalid();
96+
CleanupHandle tailCleanup = CleanupHandle::invalid();
97+
if (componentCleanup.isValid()) {
98+
eltCleanup = SGF.enterDestroyCleanup(elt);
99+
tailCleanup = SGF.enterPartialDestroyRemainingTupleCleanup(tupleAddr,
100+
formalPackType, componentIndex, indexWithinComponent);
101+
}
102+
103+
auto eltMV = ManagedValue(elt, eltCleanup);
104+
105+
// Perform the initialization. If this doesn't consume the
106+
// element value, that's fine, we'll just destroy it as part of
107+
// leaving the iteration.
108+
eltInit->copyOrInitValueInto(SGF, loc, eltMV, isInit);
109+
110+
// Deactivate the tail cleanup before continuing the loop.
111+
if (tailCleanup.isValid())
112+
SGF.Cleanups.forwardCleanup(tailCleanup);
113+
});
114+
});
115+
116+
expansionInit->finishInitialization(SGF);
117+
}
118+
52119
void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF,
53120
SILLocation loc,
54121
ManagedValue value, bool isInit) {
55-
// Process all values before initialization all at once to ensure all cleanups
56-
// are setup on all tuple elements before a potential early exit.
122+
auto sourceType = value.getType().castTo<TupleType>();
123+
assert(sourceType->getNumElements() == SubInitializations.size());
124+
125+
// We have to emit a different pattern when there are pack expansions.
126+
// Fortunately, we can assume this doesn't happen with objects because
127+
// tuples contain pack expansions are address-only.
128+
auto containsPackExpansion = sourceType.containsPackExpansionType();
129+
130+
CanPackType formalPackType;
131+
if (containsPackExpansion)
132+
formalPackType = FormalTupleType.getInducedPackType();
133+
134+
// Process all values before initialization all at once to ensure
135+
// all cleanups are setup on all tuple elements before a potential
136+
// early exit.
57137
SmallVector<ManagedValue, 8> destructuredValues;
58138

59-
// In the object case, emit a destructure operation and return.
139+
// In the object case, destructure the tuple.
60140
if (value.getType().isObject()) {
141+
assert(!containsPackExpansion);
61142
SGF.B.emitDestructureValueOperation(loc, value, destructuredValues);
62143
} else {
63144
// In the address case, we forward the underlying value and store it
@@ -67,11 +148,23 @@ void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF,
67148
CleanupCloner cloner(SGF, value);
68149
SILValue v = value.forward(SGF);
69150

70-
auto sourceType = value.getType().castTo<TupleType>();
71151
auto sourceSILType = value.getType();
72-
for (unsigned i : range(sourceType->getNumElements())) {
152+
for (auto i : range(sourceType->getNumElements())) {
73153
SILType fieldTy = sourceSILType.getTupleElementType(i);
74-
SILValue elt = SGF.B.createTupleElementAddr(loc, v, i, fieldTy);
154+
if (containsPackExpansion && fieldTy.is<PackExpansionType>()) {
155+
destructuredValues.push_back(
156+
cloner.cloneForTuplePackExpansionComponent(v, formalPackType, i));
157+
continue;
158+
}
159+
160+
SILValue elt;
161+
if (containsPackExpansion) {
162+
auto packIndex = SGF.B.createScalarPackIndex(loc, i, formalPackType);
163+
elt = SGF.B.createTuplePackElementAddr(loc, packIndex, v, fieldTy);
164+
} else {
165+
elt = SGF.B.createTupleElementAddr(loc, v, i, fieldTy);
166+
}
167+
75168
if (!fieldTy.isAddressOnly(SGF.F)) {
76169
elt = SGF.B.emitLoadValueOperation(loc, elt,
77170
LoadOwnershipQualifier::Take);
@@ -80,7 +173,24 @@ void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF,
80173
}
81174
}
82175

83-
for (unsigned i : indices(destructuredValues)) {
176+
assert(destructuredValues.size() == SubInitializations.size());
177+
178+
for (auto i : indices(destructuredValues)) {
179+
if (containsPackExpansion) {
180+
bool isPackExpansion =
181+
(destructuredValues[i].getValue() == value.getValue());
182+
assert(isPackExpansion ==
183+
isa<PackExpansionType>(sourceType.getElementType(i)));
184+
if (isPackExpansion) {
185+
auto packAddr = destructuredValues[i].getValue();
186+
auto componentCleanup = destructuredValues[i].getCleanup();
187+
copyOrInitPackExpansionInto(SGF, loc, packAddr, formalPackType,
188+
i, componentCleanup,
189+
SubInitializations[i].get(), isInit);
190+
continue;
191+
}
192+
}
193+
84194
SubInitializations[i]->copyOrInitValueInto(SGF, loc, destructuredValues[i],
85195
isInit);
86196
SubInitializations[i]->finishInitialization(SGF);
@@ -148,8 +258,7 @@ splitSingleBufferIntoTupleElements(SILGenFunction &SGF, SILLocation loc,
148258
// type for the tuple elements below.
149259
CanPackType inducedPackType;
150260
if (hasExpansion) {
151-
inducedPackType =
152-
tupleType.getInducedPackType(0, tupleType->getNumElements());
261+
inducedPackType = tupleType.getInducedPackType();
153262
}
154263

155264
// Destructure the buffer into per-element buffers.
@@ -1250,7 +1359,8 @@ struct InitializationForPattern
12501359
// Bind a tuple pattern by aggregating the component variables into a
12511360
// TupleInitialization.
12521361
InitializationPtr visitTuplePattern(TuplePattern *P) {
1253-
TupleInitialization *init = new TupleInitialization();
1362+
TupleInitialization *init = new TupleInitialization(
1363+
cast<TupleType>(P->getType()->getCanonicalType()));
12541364
for (auto &elt : P->getElements())
12551365
init->SubInitializations.push_back(visit(elt.getPattern()));
12561366
return InitializationPtr(init);

lib/SILGen/SILGenExpr.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,8 +1541,7 @@ RValueEmitter::visitPackExpansionExpr(PackExpansionExpr *E,
15411541
auto formalPackType = CanPackType::get(SGF.getASTContext(), {type});
15421542

15431543
SGF.emitDynamicPackLoop(E, formalPackType, /*component index*/ 0,
1544-
/*limit within component*/ SILValue(),
1545-
E->getGenericEnvironment(), /*reverse*/ false,
1544+
E->getGenericEnvironment(),
15461545
[&](SILValue indexWithinComponent,
15471546
SILValue packExpansionIndex,
15481547
SILValue packIndex) {
@@ -6210,8 +6209,7 @@ static void emitIgnoredPackExpansion(SILGenFunction &SGF,
62106209
auto formalPackType = CanPackType::get(SGF.getASTContext(), expansionType);
62116210
auto openedElementEnv = E->getGenericEnvironment();
62126211
SGF.emitDynamicPackLoop(E, formalPackType, /*component index*/ 0,
6213-
/*limit*/ SILValue(), openedElementEnv,
6214-
/*reverse*/ false,
6212+
openedElementEnv,
62156213
[&](SILValue indexWithinComponent,
62166214
SILValue packExpansionIndex,
62176215
SILValue packIndex) {

0 commit comments

Comments
 (0)