Skip to content

Commit 4259e4f

Browse files
rjmccallrjmccall
authored
Merge pull request #70722 from rjmccall/concrete-tuple-forwarding-as-pack
Fix expanded argument emission for tuple expressions in pack context
2 parents d36ef3f + d3058cb commit 4259e4f

File tree

4 files changed

+203
-70
lines changed

4 files changed

+203
-70
lines changed

‎lib/SILGen/ArgumentSource.cpp‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,3 +332,88 @@ ArgumentSource ArgumentSource::copyForDiagnostics() const {
332332
}
333333
llvm_unreachable("bad kind");
334334
}
335+
336+
ArgumentSourceExpansion::ArgumentSourceExpansion(SILGenFunction &SGF,
337+
ArgumentSource &&arg,
338+
bool vanishes) {
339+
if (vanishes) {
340+
StoredKind = Kind::Vanishing;
341+
Storage.emplace<ArgumentSource *>(StoredKind, &arg);
342+
#ifndef NDEBUG
343+
NumRemainingElements = 1;
344+
#endif
345+
return;
346+
}
347+
348+
#ifndef NDEBUG
349+
NumRemainingElements =
350+
cast<TupleType>(arg.getSubstRValueType())->getNumElements();
351+
#endif
352+
353+
// If we have an expression, check whether it's something we can
354+
// naturally split.
355+
assert(!arg.isLValue());
356+
Expr *expr = nullptr;
357+
if (arg.isExpr()) {
358+
expr = std::move(arg).asKnownExpr()->getSemanticsProvidingExpr();
359+
360+
// Currently, the only case of this is a tuple literal.
361+
if (auto tupleExpr = dyn_cast<TupleExpr>(expr)) {
362+
StoredKind = Kind::TupleExpr;
363+
Storage.emplace<TupleExpr*>(StoredKind, tupleExpr);
364+
return;
365+
}
366+
}
367+
368+
// Otherwise, get the arg as an r-value and extract the elements.
369+
// The location will be overwritten in the cases below.
370+
StoredKind = Kind::ElementRValues;
371+
auto &rvalues = Storage.emplace<ElementRValuesStorage>(StoredKind,
372+
SILLocation::invalid());
373+
374+
// This may require emitting the expression if we had a non-TupleExpr
375+
// expression above.
376+
if (expr) {
377+
rvalues.Loc = expr;
378+
auto rvalue = SGF.emitRValue(expr);
379+
std::move(rvalue).extractElements(rvalues.Elements);
380+
} else {
381+
rvalues.Loc = arg.getKnownRValueLocation();
382+
std::move(arg).asKnownRValue(SGF).extractElements(rvalues.Elements);
383+
}
384+
assert(rvalues.Elements.size() == NumRemainingElements);
385+
}
386+
387+
void ArgumentSourceExpansion::withElement(unsigned i,
388+
llvm::function_ref<void (ArgumentSource &&)> function) {
389+
#ifndef NDEBUG
390+
assert(NumRemainingElements > 0);
391+
NumRemainingElements--;
392+
#endif
393+
switch (StoredKind) {
394+
case Kind::ElementRValues: {
395+
auto &storage = Storage.get<ElementRValuesStorage>(StoredKind);
396+
auto &eltRV = storage.Elements[i];
397+
assert(!eltRV.isNull());
398+
function(ArgumentSource(storage.Loc, std::move(eltRV)));
399+
#ifndef NDEBUG
400+
eltRV = RValue();
401+
#endif
402+
return;
403+
}
404+
405+
case Kind::TupleExpr: {
406+
auto expr = Storage.get<TupleExpr*>(StoredKind);
407+
function(ArgumentSource(expr->getElement(i)));
408+
return;
409+
}
410+
411+
case Kind::Vanishing: {
412+
assert(NumRemainingElements == 0);
413+
auto &source = *Storage.get<ArgumentSource *>(StoredKind);
414+
function(std::move(source));
415+
return;
416+
}
417+
}
418+
llvm_unreachable("bad kind");
419+
}

‎lib/SILGen/ArgumentSource.h‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,67 @@ class PreparedArguments {
376376
PreparedArguments copyForDiagnostics() const;
377377
};
378378

379+
/// A class designed to provide a relatively optimal expansion
380+
/// of an argument source of tuple type.
381+
class ArgumentSourceExpansion {
382+
enum class Kind : uint8_t {
383+
ElementRValues,
384+
TupleExpr,
385+
Vanishing,
386+
};
387+
388+
struct ElementRValuesStorage {
389+
llvm::SmallVector<RValue, 4> Elements;
390+
SILLocation Loc;
391+
392+
ElementRValuesStorage(SILLocation loc) : Loc(loc) {}
393+
};
394+
395+
using StorageMembers = ExternalUnionMembers<ElementRValuesStorage,
396+
TupleExpr *,
397+
ArgumentSource *>;
398+
static StorageMembers::Index getStorageIndexForKind(Kind kind) {
399+
switch (kind) {
400+
case Kind::ElementRValues:
401+
return StorageMembers::indexOf<ElementRValuesStorage>();
402+
case Kind::TupleExpr:
403+
return StorageMembers::indexOf<TupleExpr*>();
404+
case Kind::Vanishing:
405+
return StorageMembers::indexOf<ArgumentSource *>();
406+
}
407+
llvm_unreachable("bad kind");
408+
}
409+
410+
ExternalUnion<Kind, StorageMembers, getStorageIndexForKind> Storage;
411+
Kind StoredKind;
412+
#ifndef NDEBUG
413+
unsigned NumRemainingElements;
414+
#endif
415+
416+
public:
417+
/// Begin an expansion of the given argument source, which usually
418+
/// must have tuple type. However, if `vanishes` is passed, the
419+
/// the argument source will *not* be expanded; the expansion behaves
420+
/// instead as if it were of a nominal singleton tuple containing
421+
/// the source. (This is very useful for dealing with vanishing tuples
422+
/// under variadic generics.)
423+
///
424+
/// The expansion may keep a reference to the argument source passed in.
425+
ArgumentSourceExpansion(SILGenFunction &SGF, ArgumentSource &&arg,
426+
bool vanishes = false);
427+
428+
ArgumentSourceExpansion(const ArgumentSourceExpansion &) = delete;
429+
ArgumentSourceExpansion &operator=(const ArgumentSourceExpansion &) = delete;
430+
431+
~ArgumentSourceExpansion() {
432+
assert(NumRemainingElements == 0 && "didn't claim all elements?");
433+
Storage.destruct(StoredKind);
434+
}
435+
436+
void withElement(unsigned i,
437+
llvm::function_ref<void (ArgumentSource &&)> function);
438+
};
439+
379440
} // end namespace Lowering
380441
} // end namespace swift
381442

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 26 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -3448,83 +3448,39 @@ class ArgEmitter {
34483448
void emitExpanded(ArgumentSource &&arg, AbstractionPattern origParamType) {
34493449
assert(!arg.isLValue() && "argument is l-value but parameter is tuple?");
34503450

3451-
// If the original parameter type is a vanishing tuple, we want to emit
3452-
// this as if the argument source was wrapped in an extra level of
3453-
// tuple literal.
3454-
bool origTupleVanishes = origParamType.doesTupleVanish();
3455-
3456-
auto substType = arg.getSubstRValueType();
3457-
3458-
// If we're working with an r-value, just expand it out and emit
3459-
// all the elements individually.
3460-
// FIXME: this code is not doing the right thing with packs
3461-
if (arg.isRValue()) {
3462-
if (CanTupleType substArgType = dyn_cast<TupleType>(substType)) {
3463-
// The original type isn't necessarily a tuple.
3464-
if (!origParamType.matchesTuple(substArgType))
3465-
origParamType = origParamType.getTupleElementType(0);
3466-
3467-
assert(origParamType.matchesTuple(substArgType));
3468-
3469-
auto loc = arg.getKnownRValueLocation();
3470-
SmallVector<RValue, 4> elts;
3471-
std::move(arg).asKnownRValue(SGF).extractElements(elts);
3472-
for (auto i : indices(substArgType.getElementTypes())) {
3473-
emit({ loc, std::move(elts[i]) },
3474-
origParamType.getTupleElementType(i));
3475-
}
3451+
// Handle yields of storage reference expressions specially so that we
3452+
// don't emit them as +1 r-values and then expand.
3453+
if (IsYield) {
3454+
if (auto result = std::move(arg).findStorageReferenceExprForBorrow()) {
3455+
emitExpandedBorrowed(result, origParamType);
34763456
return;
34773457
}
3478-
3479-
auto loc = arg.getKnownRValueLocation();
3480-
SmallVector<RValue, 1> elts;
3481-
std::move(arg).asKnownRValue(SGF).extractElements(elts);
3482-
emit({ loc, std::move(elts[0]) },
3483-
origParamType.getTupleElementType(0));
3484-
return;
3485-
}
3486-
3487-
// Otherwise, we're working with an expression.
3488-
Expr *e = std::move(arg).asKnownExpr();
3489-
3490-
// If the source expression is a tuple literal, we can break it
3491-
// up directly. We can also do this if the orig type is a vanishing
3492-
// tuple, because we want to treat that like it was the sole element
3493-
// of a tuple. Note that vanishing tuples take priority: the
3494-
// singleton element could itself be a tuple.
3495-
auto tupleExpr = dyn_cast<TupleExpr>(e);
3496-
if (origTupleVanishes || tupleExpr) {
3497-
auto getElementExpr = [&](unsigned index) {
3498-
assert(!origTupleVanishes || index == 0);
3499-
return (origTupleVanishes ? e : tupleExpr->getElement(index));
3500-
};
3501-
origParamType.forEachTupleElement(substType,
3502-
[&](TupleElementGenerator &elt) {
3503-
if (!elt.isOrigPackExpansion()) {
3504-
emit(getElementExpr(elt.getSubstIndex()), elt.getOrigType());
3505-
return;
3506-
}
3507-
3508-
auto substEltTypes = elt.getSubstTypes();
3509-
SmallVector<ArgumentSource, 4> eltArgs;
3510-
eltArgs.reserve(substEltTypes.size());
3511-
for (auto i : elt.getSubstIndexRange()) {
3512-
eltArgs.emplace_back(getElementExpr(i));
3513-
}
3514-
emitPackArg(eltArgs, elt.getOrigType());
3515-
});
3516-
return;
35173458
}
35183459

3519-
if (IsYield) {
3520-
if (auto result = findStorageReferenceExprForBorrow(e)) {
3521-
emitExpandedBorrowed(result.getTransitiveRoot(), origParamType);
3460+
auto substType = arg.getSubstRValueType();
3461+
bool doesTupleVanish = origParamType.doesTupleVanish();
3462+
3463+
ArgumentSourceExpansion expander(SGF, std::move(arg), doesTupleVanish);
3464+
origParamType.forEachTupleElement(substType,
3465+
[&](TupleElementGenerator &origElt) {
3466+
if (!origElt.isOrigPackExpansion()) {
3467+
expander.withElement(origElt.getSubstIndex(),
3468+
[&](ArgumentSource &&eltSource) {
3469+
emit(std::move(eltSource), origElt.getOrigType());
3470+
});
35223471
return;
35233472
}
3524-
}
35253473

3526-
// Fall back to the r-value case.
3527-
emitExpanded({ e, SGF.emitRValue(e) }, origParamType);
3474+
auto substTypes = origElt.getSubstTypes();
3475+
SmallVector<ArgumentSource, 8> packEltSources;
3476+
packEltSources.reserve(substTypes.size());
3477+
for (auto substEltIndex : origElt.getSubstIndexRange()) {
3478+
expander.withElement(substEltIndex, [&](ArgumentSource &&eltSource) {
3479+
packEltSources.emplace_back(std::move(eltSource));
3480+
});
3481+
}
3482+
emitPackArg(packEltSources, origElt.getOrigType());
3483+
});
35283484
}
35293485

35303486
void emitIndirect(ArgumentSource &&arg,

‎test/SILGen/variadic-generic-tuples.swift‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,3 +368,34 @@ func testExistentialErasure<each T>(_: repeat each T) {
368368

369369
g(1, "hi", false)
370370
}
371+
372+
// Issue #70187
373+
func identityOnVariadicTuples<each T>(_ value: (repeat each T)) -> (repeat each T) {
374+
(repeat each value)
375+
}
376+
377+
func testPassReturnedVariadicTuple() {
378+
takesVariadicTuple(tuple: identityOnVariadicTuples((1, 2, 3)))
379+
}
380+
// CHECK-LABEL: sil {{.*}}@$s4main29testPassReturnedVariadicTupleyyF :
381+
// CHECK: [[RESULT_PACK:%.*]] = alloc_pack $Pack{Int, Int, Int}
382+
// CHECK-NEXT: [[E0_ADDR:%.*]] = alloc_stack $Int
383+
// CHECK-NEXT: [[I0:%.*]] = scalar_pack_index 0 of $Pack{Int, Int, Int}
384+
// CHECK-NEXT: pack_element_set [[E0_ADDR]] : $*Int into [[I0]] of [[RESULT_PACK]] :
385+
// CHECK-NEXT: [[E1_ADDR:%.*]] = alloc_stack $Int
386+
// CHECK-NEXT: [[I1:%.*]] = scalar_pack_index 1 of $Pack{Int, Int, Int}
387+
// CHECK-NEXT: pack_element_set [[E1_ADDR]] : $*Int into [[I1]] of [[RESULT_PACK]] :
388+
// CHECK-NEXT: [[E2_ADDR:%.*]] = alloc_stack $Int
389+
// CHECK-NEXT: [[I2:%.*]] = scalar_pack_index 2 of $Pack{Int, Int, Int}
390+
// CHECK-NEXT: pack_element_set [[E2_ADDR]] : $*Int into [[I2]] of [[RESULT_PACK]] :
391+
// CHECK-NEXT: [[ARG_PACK:%.*]] = alloc_pack $Pack{Int, Int, Int}
392+
// CHECK: apply {{.*}}<Pack{Int, Int, Int}>([[RESULT_PACK]], [[ARG_PACK]])
393+
// CHECK: [[E0:%.*]] = load [trivial] [[E0_ADDR]] : $*Int
394+
// CHECK-NEXT: [[E1:%.*]] = load [trivial] [[E1_ADDR]] : $*Int
395+
// CHECK-NEXT: [[E2:%.*]] = load [trivial] [[E2_ADDR]] : $*Int
396+
// CHECK-NEXT: [[ARG_PACK2:%.*]] = alloc_pack $Pack{Int, Int, Int}
397+
398+
func test() {
399+
let tuple = identityOnVariadicTuples((1, 2, 3))
400+
takesVariadicTuple(tuple: tuple)
401+
}

0 commit comments

Comments
 (0)