Skip to content

Commit a9e43a7

Browse files
authored
Merge pull request #66684 from rjmccall/variadic-generics-fixes-5.9
[5.9] A few SILGen fixes for variadic generics
2 parents cb109dc + 1d9b9ab commit a9e43a7

12 files changed

+259
-54
lines changed

include/swift/SIL/AbstractionPatternGenerators.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,22 @@ class TupleElementGenerator {
291291
}
292292
}
293293

294+
/// Like `getSubstTypes`, but uses a different and possibly
295+
/// non-canonical tuple type.
296+
TupleEltTypeArrayRef getSubstTypes(Type ncSubstType) const {
297+
assert(!isFinished());
298+
if (!origTupleVanishes) {
299+
return ncSubstType->castTo<TupleType>()
300+
->getElementTypes().slice(substEltIndex,
301+
numSubstEltsForOrigElt);
302+
} else if (numSubstEltsForOrigElt == 0) {
303+
return TupleEltTypeArrayRef();
304+
} else {
305+
scratchSubstElt = TupleTypeElt(ncSubstType);
306+
return TupleEltTypeArrayRef(scratchSubstElt);
307+
}
308+
}
309+
294310
/// Call this to finalize the traversal and assert that it was done
295311
/// properly.
296312
void finish() {

lib/SIL/IR/AbstractionPattern.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2064,6 +2064,9 @@ class SubstFunctionTypePatternVisitor
20642064
if (auto gp = handleTypeParameter(pattern, t, level))
20652065
return gp;
20662066

2067+
if (pattern.isTuple())
2068+
return visitTuplePattern(t, pattern);
2069+
20672070
return CanTypeVisitor::visit(t, pattern);
20682071
}
20692072

@@ -2312,11 +2315,15 @@ class SubstFunctionTypePatternVisitor
23122315
TC.Context, ppt->getBaseType(), substArgs));
23132316
}
23142317

2315-
CanType visitTupleType(CanTupleType tuple, AbstractionPattern pattern) {
2318+
/// Visit a tuple pattern. Note that, because of vanishing tuples,
2319+
/// we can't handle this case by matching a tuple type in the
2320+
/// substituted type; we have to check for a tuple pattern in the
2321+
/// top-level visit routine.
2322+
CanType visitTuplePattern(CanType substType, AbstractionPattern pattern) {
23162323
assert(pattern.isTuple());
23172324

23182325
SmallVector<TupleTypeElt, 4> tupleElts;
2319-
pattern.forEachTupleElement(tuple, [&](TupleElementGenerator &elt) {
2326+
pattern.forEachTupleElement(substType, [&](TupleElementGenerator &elt) {
23202327
auto substEltTypes = elt.getSubstTypes();
23212328
CanType eltTy;
23222329
if (!elt.isOrigPackExpansion()) {

lib/SIL/IR/SILType.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,16 @@ bool SILType::isLoweringOf(TypeExpansionContext context, SILModule &Mod,
851851
}
852852
}
853853

854+
// The pattern of a pack expansion is lowered.
855+
if (auto formalExpansion = dyn_cast<PackExpansionType>(formalType)) {
856+
if (auto loweredExpansion = loweredType.getAs<PackExpansionType>()) {
857+
return loweredExpansion.getCountType() == formalExpansion.getCountType()
858+
&& SILType::getPrimitiveAddressType(loweredExpansion.getPatternType())
859+
.isLoweringOf(context, Mod, formalExpansion.getPatternType());
860+
}
861+
return false;
862+
}
863+
854864
// Dynamic self has the same lowering as its contained type.
855865
if (auto dynamicSelf = dyn_cast<DynamicSelfType>(formalType))
856866
formalType = dynamicSelf.getSelfType();

lib/SILGen/RValue.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,8 @@ class ExplodeTupleValue
9494

9595
void visitType(CanType formalType, ManagedValue v) {
9696
// If we have a loadable type that has not been loaded, actually load it.
97-
if (!v.getType().isObject() && v.getType().isLoadable(SGF.F)) {
98-
if (v.isPlusOne(SGF)) {
99-
v = SGF.B.createLoadTake(loc, v);
100-
} else {
101-
v = SGF.B.createLoadBorrow(loc, v);
102-
}
97+
if (!v.getType().isObject()) {
98+
v = SGF.B.createLoadIfLoadable(loc, v);
10399
}
104100

105101
values.push_back(v);

lib/SILGen/SILGenBuilder.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,23 @@ ManagedValue SILGenBuilder::createUncheckedTakeEnumDataAddr(
437437
return cloner.clone(result);
438438
}
439439

440+
ManagedValue
441+
SILGenBuilder::createLoadIfLoadable(SILLocation loc, ManagedValue addr) {
442+
assert(addr.getType().isAddress());
443+
if (!addr.getType().isLoadable(SGF.F))
444+
return addr;
445+
return createLoadWithSameOwnership(loc, addr);
446+
}
447+
448+
ManagedValue
449+
SILGenBuilder::createLoadWithSameOwnership(SILLocation loc,
450+
ManagedValue addr) {
451+
if (addr.isPlusOne(SGF))
452+
return createLoadTake(loc, addr);
453+
else
454+
return createLoadBorrow(loc, addr);
455+
}
456+
440457
ManagedValue SILGenBuilder::createLoadTake(SILLocation loc, ManagedValue v) {
441458
auto &lowering = SGF.getTypeLowering(v.getType());
442459
return createLoadTake(loc, v, lowering);

lib/SILGen/SILGenBuilder.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,15 @@ class SILGenBuilder : public SILBuilder {
236236
ManagedValue createUncheckedTakeEnumDataAddr(SILLocation loc, ManagedValue operand,
237237
EnumElementDecl *element, SILType ty);
238238

239+
/// Given the address of a value, load a scalar value from it if the type
240+
/// is loadable. Most general routines in SILGen expect to work with
241+
/// values with the canonical scalar-ness for their type.
242+
ManagedValue createLoadIfLoadable(SILLocation loc, ManagedValue addr);
243+
244+
/// Given the address of a loadable value, load the value but don't
245+
/// change the ownership.
246+
ManagedValue createLoadWithSameOwnership(SILLocation loc, ManagedValue addr);
247+
239248
ManagedValue createLoadTake(SILLocation loc, ManagedValue addr);
240249
ManagedValue createLoadTake(SILLocation loc, ManagedValue addr,
241250
const TypeLowering &lowering);

lib/SILGen/SILGenConstructor.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ static RValue emitImplicitValueConstructorArg(SILGenFunction &SGF,
222222
auto eltAddr =
223223
SGF.B.createPackElementGet(loc, packIndex, arg, eltTy);
224224
ManagedValue eltMV = emitManagedParameter(SGF, eltAddr, argIsConsumed);
225+
eltMV = SGF.B.createLoadIfLoadable(loc, eltMV);
225226
eltInit->copyOrInitValueInto(SGF, loc, eltMV, argIsConsumed);
226227
eltInit->finishInitialization(SGF);
227228
});
@@ -232,21 +233,18 @@ static RValue emitImplicitValueConstructorArg(SILGenFunction &SGF,
232233

233234
ManagedValue mvArg = emitManagedParameter(SGF, arg, argIsConsumed);
234235

236+
// This can happen if the value is resilient in the calling convention
237+
// but not resilient locally.
238+
if (argType.isAddress()) {
239+
mvArg = SGF.B.createLoadIfLoadable(loc, mvArg);
240+
}
241+
235242
if (argInit) {
236243
argInit->copyOrInitValueInto(SGF, loc, mvArg, argIsConsumed);
237244
argInit->finishInitialization(SGF);
238245
return RValue::forInContext();
239246
}
240247

241-
// This can happen if the value is resilient in the calling convention
242-
// but not resilient locally.
243-
if (argType.isLoadable(SGF.F) && argType.isAddress()) {
244-
if (mvArg.isPlusOne(SGF))
245-
mvArg = SGF.B.createLoadTake(loc, mvArg);
246-
else
247-
mvArg = SGF.B.createLoadBorrow(loc, mvArg);
248-
}
249-
250248
return RValue(SGF, loc, type, mvArg);
251249
}
252250

lib/SILGen/SILGenProlog.cpp

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,7 @@ class EmitBBArguments : public CanTypeVisitor<EmitBBArguments,
263263
bool argIsLoadable = argType.isLoadable(SGF.F);
264264
if (argIsLoadable) {
265265
if (argType.isAddress()) {
266-
if (mv.isPlusOne(SGF))
267-
mv = SGF.B.createLoadTake(loc, mv);
268-
else
269-
mv = SGF.B.createLoadBorrow(loc, mv);
266+
mv = SGF.B.createLoadWithSameOwnership(loc, mv);
270267
argType = argType.getObjectType();
271268
}
272269
}
@@ -1569,54 +1566,91 @@ SILValue SILGenFunction::emitGetCurrentExecutor(SILLocation loc) {
15691566
return ExpectedExecutor;
15701567
}
15711568

1569+
static void emitIndirectPackParameter(SILGenFunction &SGF,
1570+
PackType *resultType,
1571+
CanTupleEltTypeArrayRef
1572+
resultTypesInContext,
1573+
AbstractionPattern origExpansionType,
1574+
DeclContext *DC) {
1575+
auto &ctx = SGF.getASTContext();
1576+
1577+
bool indirect =
1578+
origExpansionType.arePackElementsPassedIndirectly(SGF.SGM.Types);
1579+
SmallVector<CanType, 4> packElts;
1580+
for (auto substEltType : resultTypesInContext) {
1581+
auto origComponentType
1582+
= origExpansionType.getPackExpansionComponentType(substEltType);
1583+
CanType loweredEltTy =
1584+
SGF.getLoweredRValueType(origComponentType, substEltType);
1585+
packElts.push_back(loweredEltTy);
1586+
}
1587+
1588+
SILPackType::ExtInfo extInfo(indirect);
1589+
auto packType = SILPackType::get(ctx, extInfo, packElts);
1590+
auto resultSILType = SILType::getPrimitiveAddressType(packType);
1591+
1592+
auto var = new (ctx) ParamDecl(SourceLoc(), SourceLoc(),
1593+
ctx.getIdentifier("$return_value"), SourceLoc(),
1594+
ctx.getIdentifier("$return_value"),
1595+
DC);
1596+
var->setSpecifier(ParamSpecifier::InOut);
1597+
var->setInterfaceType(resultType);
1598+
auto *arg = SGF.F.begin()->createFunctionArgument(resultSILType, var);
1599+
(void)arg;
1600+
}
1601+
15721602
static void emitIndirectResultParameters(SILGenFunction &SGF,
15731603
Type resultType,
15741604
AbstractionPattern origResultType,
15751605
DeclContext *DC) {
1576-
// Expand tuples.
1606+
CanType resultTypeInContext =
1607+
DC->mapTypeIntoContext(resultType)->getCanonicalType();
1608+
1609+
// Tuples in the original result type are expanded.
15771610
if (origResultType.isTuple()) {
1578-
auto tupleType = resultType->castTo<TupleType>();
1579-
for (unsigned i = 0, e = origResultType.getNumTupleElements(); i < e; ++i) {
1580-
emitIndirectResultParameters(SGF, tupleType->getElementType(i),
1581-
origResultType.getTupleElementType(i),
1582-
DC);
1583-
}
1611+
origResultType.forEachTupleElement(resultTypeInContext,
1612+
[&](TupleElementGenerator &elt) {
1613+
auto origEltType = elt.getOrigType();
1614+
auto substEltTypes = elt.getSubstTypes(resultType);
1615+
1616+
// If the original element isn't a pack expansion, pull out the
1617+
// corresponding substituted tuple element and recurse.
1618+
if (!elt.isOrigPackExpansion()) {
1619+
emitIndirectResultParameters(SGF, substEltTypes[0], origEltType, DC);
1620+
return;
1621+
}
1622+
1623+
// Otherwise, bind a pack parameter.
1624+
PackType *resultPackType = [&] {
1625+
SmallVector<Type, 4> packElts(substEltTypes.begin(),
1626+
substEltTypes.end());
1627+
return PackType::get(SGF.getASTContext(), packElts);
1628+
}();
1629+
emitIndirectPackParameter(SGF, resultPackType, elt.getSubstTypes(),
1630+
origEltType, DC);
1631+
});
15841632
return;
15851633
}
15861634

1635+
assert(!resultType->is<PackExpansionType>());
1636+
15871637
// If the return type is address-only, emit the indirect return argument.
15881638
auto &resultTI =
1589-
SGF.SGM.Types.getTypeLowering(origResultType,
1590-
DC->mapTypeIntoContext(resultType),
1639+
SGF.SGM.Types.getTypeLowering(origResultType, resultTypeInContext,
15911640
SGF.getTypeExpansionContext());
15921641

15931642
// The calling convention always uses minimal resilience expansion.
15941643
auto &resultTIConv = SGF.SGM.Types.getTypeLowering(
1595-
DC->mapTypeIntoContext(resultType), TypeExpansionContext::minimal());
1644+
resultTypeInContext, TypeExpansionContext::minimal());
15961645
auto resultConvType = resultTIConv.getLoweredType();
15971646

15981647
auto &ctx = SGF.getASTContext();
15991648

16001649
SILType resultSILType = resultTI.getLoweredType().getAddressType();
16011650

1602-
// FIXME: respect susbtitution properly and collect the appropriate
1603-
// tuple components from resultType that correspond to the
1604-
// pack expansion in origType.
1605-
bool isPackExpansion = resultType->is<PackExpansionType>();
1606-
if (isPackExpansion) {
1607-
resultType = PackType::get(ctx, {resultType});
1608-
1609-
bool indirect =
1610-
origResultType.arePackElementsPassedIndirectly(SGF.SGM.Types);
1611-
SILPackType::ExtInfo extInfo(indirect);
1612-
resultSILType = SILType::getPrimitiveAddressType(
1613-
SILPackType::get(ctx, extInfo, {resultSILType.getASTType()}));
1614-
}
1615-
16161651
// And the abstraction pattern may force an indirect return even if the
16171652
// concrete type wouldn't normally be returned indirectly.
1618-
if (!isPackExpansion &&
1619-
!SILModuleConventions::isReturnedIndirectlyInSIL(resultConvType,
1653+
if (!SILModuleConventions::isReturnedIndirectlyInSIL(resultConvType,
16201654
SGF.SGM.M)) {
16211655
if (!SILModuleConventions(SGF.SGM.M).useLoweredAddresses()
16221656
|| origResultType.getResultConvention(SGF.SGM.Types) != AbstractionPattern::Indirect)

lib/SILGen/SILGenStmt.cpp

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -580,11 +580,29 @@ prepareIndirectResultInit(SILGenFunction &SGF, SILLocation loc,
580580
SmallVectorImpl<CleanupHandle> &cleanups) {
581581
// Recursively decompose tuple abstraction patterns.
582582
if (origResultType.isTuple()) {
583-
auto resultTupleType = cast<TupleType>(resultType);
584-
auto tupleInit = new TupleInitialization(resultTupleType);
585-
tupleInit->SubInitializations.reserve(resultTupleType->getNumElements());
583+
// Normally, we build a compound initialization for the tuple. But
584+
// the initialization we build should match the substituted type,
585+
// so if the tuple in the abstraction pattern vanishes under variadic
586+
// substitution, we actually just want to return the initializer
587+
// for the surviving component.
588+
TupleInitialization *tupleInit = nullptr;
589+
SmallVector<InitializationPtr, 1> singletonEltInit;
590+
591+
bool vanishes =
592+
origResultType.getVanishingTupleElementPatternType().hasValue();
593+
if (!vanishes) {
594+
auto resultTupleType = cast<TupleType>(resultType);
595+
tupleInit = new TupleInitialization(resultTupleType);
596+
tupleInit->SubInitializations.reserve(
597+
cast<TupleType>(resultType)->getNumElements());
598+
}
599+
600+
// The list of element initializers to build into.
601+
auto &eltInits = (vanishes
602+
? static_cast<SmallVectorImpl<InitializationPtr> &>(singletonEltInit)
603+
: tupleInit->SubInitializations);
586604

587-
origResultType.forEachTupleElement(resultTupleType,
605+
origResultType.forEachTupleElement(resultType,
588606
[&](TupleElementGenerator &elt) {
589607
if (!elt.isOrigPackExpansion()) {
590608
auto eltInit = prepareIndirectResultInit(SGF, loc, fnTypeForResults,
@@ -594,7 +612,7 @@ prepareIndirectResultInit(SILGenFunction &SGF, SILLocation loc,
594612
directResults,
595613
indirectResultAddrs,
596614
cleanups);
597-
tupleInit->SubInitializations.push_back(std::move(eltInit));
615+
eltInits.push_back(std::move(eltInit));
598616
} else {
599617
assert(allResults[0].isPack());
600618
assert(SGF.silConv.isSILIndirect(allResults[0]));
@@ -604,11 +622,17 @@ prepareIndirectResultInit(SILGenFunction &SGF, SILLocation loc,
604622
indirectResultAddrs = indirectResultAddrs.slice(1);
605623

606624
preparePackResultInit(SGF, loc, elt.getOrigType(), elt.getSubstTypes(),
607-
packAddr,
608-
cleanups, tupleInit->SubInitializations);
625+
packAddr, cleanups, eltInits);
609626
}
610627
});
611628

629+
if (vanishes) {
630+
assert(singletonEltInit.size() == 1);
631+
return std::move(singletonEltInit.front());
632+
}
633+
634+
assert(tupleInit);
635+
assert(eltInits.size() == cast<TupleType>(resultType)->getNumElements());
612636
return InitializationPtr(tupleInit);
613637
}
614638

test/SILGen/variadic-generic-closures.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,12 @@ public struct UsesG<each Input> {
1212
public init<E>(builder: (repeat G<each Input>) -> E) {}
1313
}
1414
UsesG<Int, String, Bool> { a, b, c in 0 }
15+
16+
// rdar://107367324
17+
func returnVariadicClosure<each T, R>(f: @escaping (repeat each T) -> R) -> (repeat each T) -> R {
18+
{ (t: repeat each T) -> R in
19+
let tuple = (repeat each t)
20+
print(tuple)
21+
return f(repeat each t)
22+
}
23+
}

0 commit comments

Comments
 (0)