From 26255549d65ae3359ee070475c1e56927d4361b3 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 15 Dec 2025 19:09:51 -0500 Subject: [PATCH 1/3] SILArgumentConvention: fix const, add isPackParameter --- include/swift/SIL/SILArgumentConvention.h | 27 +++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/include/swift/SIL/SILArgumentConvention.h b/include/swift/SIL/SILArgumentConvention.h index fe0a266994679..c70161c61d85a 100644 --- a/include/swift/SIL/SILArgumentConvention.h +++ b/include/swift/SIL/SILArgumentConvention.h @@ -167,7 +167,7 @@ struct SILArgumentConvention { } /// Returns true if \p Value is a non-aliasing indirect parameter. - bool isExclusiveIndirectParameter() { + bool isExclusiveIndirectParameter() const { switch (Value) { case SILArgumentConvention::Indirect_In: case SILArgumentConvention::Indirect_Out: @@ -190,7 +190,7 @@ struct SILArgumentConvention { } /// Returns true if \p Value is an indirect-out parameter. - bool isIndirectOutParameter() { + bool isIndirectOutParameter() const { switch (Value) { case SILArgumentConvention::Indirect_Out: case SILArgumentConvention::Pack_Out: @@ -211,6 +211,29 @@ struct SILArgumentConvention { } llvm_unreachable("covered switch isn't covered?!"); } + + /// Returns true if \p Value is a pack parameter. + bool isPackParameter() const { + switch (Value) { + case SILArgumentConvention::Pack_Inout: + case SILArgumentConvention::Pack_Owned: + case SILArgumentConvention::Pack_Guaranteed: + case SILArgumentConvention::Pack_Out: + return true; + + case SILArgumentConvention::Indirect_Out: + case SILArgumentConvention::Indirect_In: + case SILArgumentConvention::Indirect_In_Guaranteed: + case SILArgumentConvention::Indirect_Inout: + case SILArgumentConvention::Indirect_InoutAliasable: + case SILArgumentConvention::Indirect_In_CXX: + case SILArgumentConvention::Direct_Unowned: + case SILArgumentConvention::Direct_Guaranteed: + case SILArgumentConvention::Direct_Owned: + return false; + } + llvm_unreachable("covered switch isn't covered?!"); + } }; } // namespace swift From b5170571ee718589c4fcad4d6aca9dd0e719be1e Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 15 Dec 2025 19:11:34 -0500 Subject: [PATCH 2/3] Add a helper function to SILGen to emit a loop over a managed pack expansion component. Use this in emitPackTransform; this reorders a couple of projections, but that's fine, and the result is much cleaner and has better separation of concerns. --- lib/SILGen/SILGenFunction.h | 16 +++++ lib/SILGen/SILGenPack.cpp | 84 +++++++++++++++-------- test/SILGen/protocol_packs.swift | 2 +- test/SILGen/variadic-generic-tuples.swift | 2 +- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 9d6c12c6994b7..6f75ef179f455 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -3207,6 +3207,22 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction SILValue packExpansionIndex, SILValue packIndex)> emitBody); + /// Emit an operation for each element of a pack expansion component of + /// a pack, automatically projecting and managing ownership of it properly + /// for each iteration. + /// + /// The projection and management does not itself generate control flow and + /// so can be safely composed with further projection and management. + void emitPackForEach(SILLocation loc, + ManagedValue inputPackAddr, + CanPackType inputFormalPackType, + unsigned inputComponentIndex, + GenericEnvironment *openedElementEnv, + SILType inputEltTy, + llvm::function_ref emitBody); + /// Emit a transform on each element of a pack-expansion component /// of a pack, write the result into a pack-expansion component of /// another pack. diff --git a/lib/SILGen/SILGenPack.cpp b/lib/SILGen/SILGenPack.cpp index a7dabb7a0ca48..00efc278fc20e 100644 --- a/lib/SILGen/SILGenPack.cpp +++ b/lib/SILGen/SILGenPack.cpp @@ -1027,6 +1027,51 @@ TuplePackExpansionInitialization::getElementAddress(SILGenFunction &SGF, return SGF.B.createTuplePackElementAddr(loc, packIndex, TupleAddr, eltAddrTy); } +void +SILGenFunction::emitPackForEach(SILLocation loc, + ManagedValue inputPackMV, + CanPackType inputFormalPackType, + unsigned inputComponentIndex, + GenericEnvironment *openedEnv, + SILType inputEltTy, + llvm::function_ref emitBody) { + // Deactivate the cleanup on the pack expansion component, but + // remember how to clone it for the elements later. + CleanupCloner inputCloner(*this, inputPackMV); + bool inputHasCleanup = inputPackMV.hasCleanup(); + auto inputPackAddr = inputPackMV.forward(*this); + + emitDynamicPackLoop(loc, inputFormalPackType, inputComponentIndex, openedEnv, + []() -> SILBasicBlock * { return nullptr; }, + [&](SILValue indexWithinComponent, + SILValue packExpansionIndex, + SILValue inputPackIndex) { + // Enter a partial cleanup for the remaining elements of the input + // expansion component. + CleanupHandle remainingInputEltsCleanup = CleanupHandle::invalid(); + if (inputHasCleanup) { + remainingInputEltsCleanup = + enterPartialDestroyRemainingPackCleanup( + inputPackAddr, inputFormalPackType, inputComponentIndex, + indexWithinComponent); + } + + // Retrieve the input value from the pack and manage it. + auto inputEltAddr = + B.createPackElementGet(loc, inputPackIndex, inputPackAddr, inputEltTy); + ManagedValue inputElt = inputCloner.clone(inputEltAddr); + + // Emit the body. + emitBody(indexWithinComponent, packExpansionIndex, inputElt); + + // Deactivate the partial cleanup. + if (remainingInputEltsCleanup.isValid()) + Cleanups.forwardCleanup(remainingInputEltsCleanup); + }); +} + ManagedValue SILGenFunction::emitPackTransform(SILLocation loc, ManagedValue inputPackMV, @@ -1046,11 +1091,7 @@ SILGenFunction::emitPackTransform(SILLocation loc, assert((isSimpleProjection || canForwardOutput) && "we cannot support complex transformations that yield borrows"); - CleanupCloner inputCloner(*this, inputPackMV); - bool inputHasCleanup = inputPackMV.hasCleanup(); - auto inputPackAddr = inputPackMV.forward(*this); - - auto inputPackTy = inputPackAddr->getType().castTo(); + auto inputPackTy = inputPackMV.getType().castTo(); assert(inputPackTy->getNumElements() == inputFormalPackType->getNumElements()); auto inputComponentTy = inputPackTy->getSILElementType(inputComponentIndex); @@ -1080,22 +1121,12 @@ SILGenFunction::emitPackTransform(SILLocation loc, outputTupleAddr = emitTemporaryAllocation(loc, outputTupleTy); } - emitDynamicPackLoop(loc, inputFormalPackType, inputComponentIndex, openedEnv, - []() -> SILBasicBlock * { return nullptr; }, - [&](SILValue indexWithinComponent, - SILValue packExpansionIndex, - SILValue inputPackIndex) { - // Enter a cleanup for the remaining elements of the input - // expansion component. - CleanupHandle remainingInputEltsCleanup = CleanupHandle::invalid(); - if (inputHasCleanup) { - remainingInputEltsCleanup = - enterPartialDestroyRemainingPackCleanup( - inputPackAddr, inputFormalPackType, inputComponentIndex, - indexWithinComponent); - } - - // Enter a cleanup for the previous elements of the output + emitPackForEach(loc, inputPackMV, inputFormalPackType, inputComponentIndex, + openedEnv, inputEltTy, + [&](SILValue indexWithinComponent, + SILValue packExpansionIndex, + ManagedValue inputElt) { + // Enter a partial cleanup for the previous elements of the output // expansion component. CleanupHandle previousOutputEltsCleanup = CleanupHandle::invalid(); if (outputNeedsCleanup) { @@ -1116,16 +1147,11 @@ SILGenFunction::emitPackTransform(SILLocation loc, outputEltInit = useBufferAsTemporary(outputEltAddr, outputEltTL); } - // Retrieve the input value from the pack and manage it. - auto inputEltAddr = - B.createPackElementGet(loc, inputPackIndex, inputPackAddr, inputEltTy); - ManagedValue inputElt = inputCloner.clone(inputEltAddr); - // Apply the transform. ManagedValue outputElt = emitBody(inputElt, outputEltTy, canForwardOutput ? SGFContext(outputEltInit.get()) - : SGFContext::AllowGuaranteedPlusZero); + : SGFContext::AllowGuaranteedPlusZero); assert(canForwardOutput == (outputElt.isInContext() || outputElt.isPlusOneOrTrivial(*this)) && "transformation produced a value of the wrong ownership"); @@ -1156,9 +1182,7 @@ SILGenFunction::emitPackTransform(SILLocation loc, } B.createPackElementSet(loc, outputEltAddr, outputPackIndex, outputPackAddr); - // Deactivate the partial cleanups. - if (remainingInputEltsCleanup.isValid()) - Cleanups.forwardCleanup(remainingInputEltsCleanup); + // Deactivate the partial cleanup. if (previousOutputEltsCleanup.isValid()) Cleanups.forwardCleanup(previousOutputEltsCleanup); }); diff --git a/test/SILGen/protocol_packs.swift b/test/SILGen/protocol_packs.swift index 31811c11b9e75..4db6548e28118 100644 --- a/test/SILGen/protocol_packs.swift +++ b/test/SILGen/protocol_packs.swift @@ -225,8 +225,8 @@ struct ConsumingBorrowingAccepterImpl: BorrowingAccepter { // CHECK: bb2: // CHECK-NEXT: [[PACK_INDEX:%.*]] = dynamic_pack_index [[INDEX]] of $Pack{repeat NonTrivialGeneric} // CHECK-NEXT: open_pack_element [[PACK_INDEX]] of at , shape $each τ_0_0, uuid [[UUID:".*"]] -// CHECK-NEXT: [[TEMP_ELT_ADDR:%.*]] = tuple_pack_element_addr [[PACK_INDEX]] of [[TEMP_TUPLE]] as $*NonTrivialGeneric<@pack_element([[UUID]]) each τ_0_0> // CHECK-NEXT: [[ARG_ADDR:%.*]] = pack_element_get [[PACK_INDEX]] of %0 as $*NonTrivialGeneric<@pack_element([[UUID]]) each τ_0_0> +// CHECK-NEXT: [[TEMP_ELT_ADDR:%.*]] = tuple_pack_element_addr [[PACK_INDEX]] of [[TEMP_TUPLE]] as $*NonTrivialGeneric<@pack_element([[UUID]]) each τ_0_0> // CHECK-NEXT: [[BORROW:%.*]] = load_borrow [[ARG_ADDR]] // CHECK-NEXT: [[COPY:%.*]] = copy_value [[BORROW]] // CHECK-NEXT: store [[COPY]] to [init] [[TEMP_ELT_ADDR]] diff --git a/test/SILGen/variadic-generic-tuples.swift b/test/SILGen/variadic-generic-tuples.swift index 9bcf744c5e5cf..eb045f16b3ff2 100644 --- a/test/SILGen/variadic-generic-tuples.swift +++ b/test/SILGen/variadic-generic-tuples.swift @@ -88,8 +88,8 @@ public struct Container { // CHECK: bb2: // CHECK-NEXT: [[INDEX:%.*]] = dynamic_pack_index [[IDX]] of $Pack{repeat Stored} // CHECK-NEXT: open_pack_element [[INDEX]] of at , shape $each T, uuid [[UUID:".*"]] -// CHECK-NEXT: [[ARG_COPY_ELT_ADDR:%.*]] = tuple_pack_element_addr [[INDEX]] of [[ARG_COPY]] : $*(repeat Stored) as $*Stored<@pack_element([[UUID]]) each T> // CHECK-NEXT: [[PACK_ELT_ADDR:%.*]] = pack_element_get [[INDEX]] of %0 : $*Pack{repeat Stored} as $*Stored<@pack_element([[UUID]]) each T> +// CHECK-NEXT: [[ARG_COPY_ELT_ADDR:%.*]] = tuple_pack_element_addr [[INDEX]] of [[ARG_COPY]] : $*(repeat Stored) as $*Stored<@pack_element([[UUID]]) each T> // CHECK-NEXT: [[ELT_VALUE:%.*]] = load [trivial] [[PACK_ELT_ADDR]] : $*Stored<@pack_element([[UUID]]) each T> // CHECK-NEXT: store [[ELT_VALUE]] to [trivial] [[ARG_COPY_ELT_ADDR]] : $*Stored<@pack_element([[UUID]]) each T> // CHECK-NEXT: [[NEXT_IDX:%.*]] = builtin "add_Word"([[IDX]] : $Builtin.Word, [[ONE]] : $Builtin.Word) : $Builtin.Word From 55be004af9495afa94581354faaf4e72339b98c1 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 15 Dec 2025 19:14:44 -0500 Subject: [PATCH 3/3] Fix a bug with vanishing tuples in parameter binding. This only happens when binding parameters under an abstraction pattern and can therefore only currently happen with closures passed to functions that take variadic generic tuples (that happen to vanish). This is relatively straightforward using ExpandedTupleInputGenerator, and it cleans the code up a fair bit. --- lib/SILGen/SILGenProlog.cpp | 413 ++++++++++-------- test/SILGen/let_decls.swift | 14 +- test/SILGen/tuples.swift | 2 +- .../variadic-generic-vanishing-tuples.swift | 31 ++ test/SILOptimizer/init_accessors.swift | 19 +- 5 files changed, 276 insertions(+), 203 deletions(-) diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index afc7db4b4684d..7ff4c7a77e7c9 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -17,6 +17,7 @@ #include "ManagedValue.h" #include "SILGenFunction.h" #include "Scope.h" +#include "TupleGenerators.h" #include "swift/AST/CanTypeVisitor.h" #include "swift/AST/DiagnosticsSIL.h" @@ -99,6 +100,8 @@ struct LoweredParamGenerator { bool isNoImplicitCopy = false; LifetimeAnnotation lifetimeAnnotation = LifetimeAnnotation::None; bool isImplicitParameter = false; + SILArgumentConvention currentConvention + = SILArgumentConvention::Direct_Guaranteed; // will be overwritten void configureParamData(ParamDecl *paramDecl, bool isNoImplicitCopy, LifetimeAnnotation lifetimeAnnotation) { @@ -131,9 +134,21 @@ struct LoweredParamGenerator { ManagedValue mv = SGF.B.createInputFunctionArgument( paramType, paramDecl, isNoImplicitCopy, lifetimeAnnotation, /*isClosureCapture*/ false, isFormalParameterPack, isImplicitParameter); + currentConvention = + cast(SGF.F.begin()->getArguments().back()) + ->getArgumentConvention(); return mv; } + SILArgumentConvention getCurrentConvention() const { + // This is only actually called after building a pack parameter. + assert(currentConvention.isPackParameter()); + return currentConvention; + } + void finishCurrent(ManagedValue packAddr) { + // Ignore: we don't care about residual cleanups on the pack. + } + std::optional peek() const { if (isFinished()) return {}; @@ -186,11 +201,7 @@ struct WritebackReabstractedInoutCleanup final : Cleanup { } }; -class EmitBBArguments : public CanTypeVisitor -{ +class EmitBBArguments { public: SILGenFunction &SGF; SILLocation loc; @@ -216,7 +227,26 @@ class EmitBBArguments : public CanTypeVisitor(substType)) { + return handleScalar(origType, + CanPackType::get(SGF.getASTContext(), {substType}), + /*emitInto*/ nullptr); + } + + return handleValue(origType, substType, /*emitInto*/ nullptr); + } + + ManagedValue handleValue(AbstractionPattern origType, CanType substType, + Initialization *emitInto) { + if (origType.isTuple()) { + return handleExpansion(origType, substType, emitInto); + } + return handleScalar(origType, substType, emitInto); } ManagedValue handlePackComponent(FunctionInputGenerator &formalParam) { @@ -287,8 +317,8 @@ class EmitBBArguments : public CanTypeVisitorgetCanonicalType(), - orig, emitInto); - } - - ManagedValue visitTupleType(CanTupleType t, AbstractionPattern orig, - Initialization *emitInto) { - // Only destructure if the abstraction pattern is also a tuple. - if (!orig.isTuple()) - return visitType(t, orig, emitInto); - - auto &tl = SGF.SGM.Types.getTypeLowering(t, SGF.getTypeExpansionContext()); + static bool shouldForceIntoTemporary(const TypeLowering &tl) { + // If the type is loadable, we never need this. + if (tl.isLoadable()) return false; - // If the tuple contains pack expansions, and we're not emitting - // into an initialization already, create a temporary so that we're - // always emitting into an initialization. - if (t.containsPackExpansionType() && !emitInto) { - auto temporary = SGF.emitTemporary(loc, tl); - auto result = expandTuple(orig, t, tl, temporary.get()); - assert(result.isInContext()); (void) result; + // Otherwise, we have an address-only type. + return false; + } - return temporary->getManagedAddress(); - } + ManagedValue handleExpansion(AbstractionPattern origType, CanType substType, + Initialization *init) { + assert(origType.isTuple()); + + auto &substTL = + SGF.SGM.Types.getTypeLowering(substType, SGF.getTypeExpansionContext()); + + ExpandedTupleInputGenerator expansion(SGF.getASTContext(), parameters, + origType, substType); + + // If the original tuple vanishes, the type lowering and initialization + // are actually for its surviving element. + bool tupleVanishes = expansion.doesOrigTupleVanish(); + + // If we're not already emitting into an initialization, there are + // several situations where we want to do so. + bool emitIntoTemporary = [&] { + if (init) return false; + + // We don't need to do this if the orig tuple vanishes. (It may end up + // being a tuple in the substituted type, but we should defer the decision + // to create a temporary; the tuple might end up being passed as a single + // pack element.) + if (tupleVanishes) return false; + + // All of the situations that want a temporary are exclusive with + // having a loadable tuple. + if (substTL.isLoadable()) return false; + + // If the substituted type contains pack expansions, we always need a + // a temporary (even with SIL opaque values) because we currently have + // no way of directly generating scalar tuples that contain expansions. + if (auto tupleTy = substTL.getLoweredType().getAs()) { + if (tupleTy.containsPackExpansionType()) + return true; + } - return expandTuple(orig, t, tl, emitInto); - } + // If SIL opaque values are disabled, we always need a temporary to + // reconstitute an address-only tuple. + if (SGF.silConv.useLoweredAddresses()) + return true; - ManagedValue expandTuple(AbstractionPattern orig, CanTupleType t, - const TypeLowering &tl, Initialization *init) { - assert((!t.containsPackExpansionType() || init) && - "should always have an emission context when expanding " - "a tuple containing pack expansions"); + // Otherwise, we don't need a temporary. + return false; + }(); - bool canBeGuaranteed = tl.isLoadable(); + TemporaryInitializationPtr temporary; + if (emitIntoTemporary) { + temporary = SGF.emitTemporary(loc, substTL); + init = temporary.get(); + } - // We only use specific initializations here that can always be split. + // If we're emitting into an initialization, and the tuple doesn't + // vanish, split the tuple initialization. We're only ever given + // specific kinds of initialization that can always be split. SmallVector eltInitsBuffer; MutableArrayRef eltInits; - if (init) { + if (init && !tupleVanishes) { assert(init->canSplitIntoTupleElements()); - eltInits = init->splitIntoTupleElements(SGF, loc, t, eltInitsBuffer); + eltInits = init->splitIntoTupleElements(SGF, loc, substType, + eltInitsBuffer); } - // Collect the exploded elements. - // - // Reabstraction can give us original types that are pack - // expansions without having pack expansions in the result. - // In this case, we do not need to force emission into a pack - // expansion. - SmallVector elements; - orig.forEachTupleElement(t, [&](TupleElementGenerator &elt) { - auto origEltType = elt.getOrigType(); - auto substEltTypes = elt.getSubstTypes(); - if (!elt.isOrigPackExpansion()) { - auto eltValue = - visit(substEltTypes[0], origEltType, - init ? eltInits[elt.getSubstIndex()].get() : nullptr); - assert((init != nullptr) == (eltValue.isInContext())); - if (!eltValue.isInContext()) - elements.push_back(eltValue); - - if (eltValue.hasCleanup()) - canBeGuaranteed = false; - } else { - assert(init); - expandPack(origEltType, substEltTypes, elt.getSubstIndex(), - eltInits.slice(elt.getSubstIndex(), substEltTypes.size()), - elements); + // Iterate over the elements in order, either emitting them into the + // initialization or collecting scalar values from them. + auto expectedNumElts = (init ? 0 : + tupleVanishes ? 1 : + cast(substType)->getNumElements()); + SmallVector eltMVs; + eltMVs.reserve(expectedNumElts); + + // Remember if we saw anything that would force us to copy in order to + // form the tuple: + // - if any of the scalar components have cleanups, since we can't mix + // owned and guaranteed values in the tuple, or + // - if the tuple overall is address-only, since that's a representation + // change that the address lowering can't reliably eliminate without + // ownership. + // This is only used in the path where we construct a tuple. + bool tupleValueCanBeGuaranteed = substTL.isLoadable(); + + for (; !expansion.isFinished(); expansion.advance()) { + Initialization *componentInit = nullptr; + if (init) { + if (tupleVanishes) { + componentInit = init; + } else { + componentInit = eltInits[expansion.getSubstElementIndex()].get(); + } } - }); - - // If we emitted into a context, we're done. - if (init) { - init->finishInitialization(SGF); - return ManagedValue::forInContext(); - } - if (tl.isLoadable() || !SGF.silConv.useLoweredAddresses()) { - SmallVector elementValues; - if (canBeGuaranteed) { - // If all of the elements were guaranteed, we can form a guaranteed tuple. - for (auto element : elements) - elementValues.push_back(element.getUnmanagedValue()); - } else { - // Otherwise, we need to move or copy values into a +1 tuple. - for (auto element : elements) { - SILValue value = element.hasCleanup() - ? element.forward(SGF) - : element.copyUnmanaged(SGF, loc).forward(SGF); - elementValues.push_back(value); + // If the original element is not from a pack expansion, just + // recursively collect it. + if (!expansion.isOrigPackExpansion()) { + auto eltResult = + handleValue(expansion.getOrigType(), expansion.getSubstType(), + componentInit); + assert((init != nullptr) == (eltResult.isInContext())); + + if (!eltResult.isInContext()) { + eltMVs.push_back(eltResult); + if (eltResult.hasCleanup()) + tupleValueCanBeGuaranteed = false; } + + continue; } - auto tupleValue = SGF.B.createTuple(loc, tl.getLoweredType(), - elementValues); - if (tupleValue->getOwnershipKind() == OwnershipKind::None) - return ManagedValue::forObjectRValueWithoutOwnership(tupleValue); - return canBeGuaranteed ? ManagedValue::forBorrowedObjectRValue(tupleValue) - : SGF.emitManagedRValueWithCleanup(tupleValue); - } else { - // If the type is address-only, we need to move or copy the elements into - // a tuple in memory. - // TODO: It would be a bit more efficient to use a preallocated buffer - // in this case. - auto buffer = SGF.emitTemporaryAllocation(loc, tl.getLoweredType()); - for (auto i : indices(elements)) { - auto element = elements[i]; - auto elementBuffer = SGF.B.createTupleElementAddr(loc, buffer, - i, element.getType().getAddressType()); - if (element.hasCleanup()) - element.forwardInto(SGF, loc, elementBuffer); - else - element.copyInto(SGF, loc, elementBuffer); - } - return SGF.emitManagedRValueWithCleanup(buffer); - } - } - void expandPack(AbstractionPattern origExpansionType, - CanTupleEltTypeArrayRef substEltTypes, - size_t firstSubstEltIndex, - MutableArrayRef eltInits, - SmallVectorImpl &eltMVs) { - assert(substEltTypes.size() == eltInits.size()); - - // The next parameter is a pack which corresponds to some number of - // components in the tuple. Some of them may be pack expansions. - // Either copy/move them into the tuple (necessary if there are any - // pack expansions) or collect them in eltMVs. - - // Claim the next parameter, remember whether it was +1, and forward - // the cleanup. We can get away with just forwarding the cleanup - // up front, not destructuring it, because we assume that the work - // we're doing here won't ever unwind. - ManagedValue packAddrMV = claimNextParameter(); - CleanupCloner cloner(SGF, packAddrMV); - SILValue packAddr = packAddrMV.forward(SGF); - auto packTy = packAddr->getType().castTo(); - - auto origPatternType = origExpansionType.getPackExpansionPatternType(); - - auto inducedPackType = - CanPackType::get(SGF.getASTContext(), substEltTypes); - - for (auto packComponentIndex : indices(substEltTypes)) { - CanType substComponentType = substEltTypes[packComponentIndex]; - Initialization *componentInit = - eltInits.empty() ? nullptr : eltInits[packComponentIndex].get(); - auto packComponentTy = packTy->getSILElementType(packComponentIndex); - - auto substExpansionType = - dyn_cast(substComponentType); - - // In the scalar case, project out the element address from the - // pack and use the normal scalar path to trigger initialization. - if (!substExpansionType) { - auto packIndex = - SGF.B.createScalarPackIndex(loc, packComponentIndex, inducedPackType); - auto eltAddr = - SGF.B.createPackElementGet(loc, packIndex, packAddr, - packComponentTy); - auto eltAddrMV = cloner.clone(eltAddr); - auto result = handleScalar(eltAddrMV, origPatternType, - substComponentType, componentInit, - /*inout*/ false, - /*addressable*/ false); - assert(result.isInContext() == (componentInit != nullptr)); - if (!result.isInContext()) - eltMVs.push_back(result); + // Otherwise, we're starting with a pack parameter. Project out + // the substituted component. + auto componentMV = expansion.projectPackComponent(SGF, loc); + + // If the substituted element type is not a pack expansion, + // we can just use the substituted component in the scalar path. + if (!expansion.isSubstPackExpansion()) { + auto eltResult = handleScalar(componentMV, expansion.getOrigType(), + expansion.getSubstType(), componentInit, + /*inout*/ false, + /*addressable*/ false); + assert((init != nullptr) == (eltResult.isInContext())); + + if (!eltResult.isInContext()) { + eltMVs.push_back(eltResult); + if (eltResult.hasCleanup()) + tupleValueCanBeGuaranteed = false; + } + continue; } - // In the pack-expansion case, do the exact same thing, - // but in a pack loop. + // Otherwise, we need to emit a pack loop over the expansion + // component. Since the substituted type always contains a pack + // expansion, we should've forced the existence of an init earlier. assert(componentInit); assert(componentInit->canPerformPackExpansionInitialization()); + auto packComponentIndex = expansion.getPackComponentIndex(); + auto packComponentTy = expansion.getPackComponentType(); + auto substComponentType = expansion.getSubstType(); + SILType eltTy; CanType substEltType; auto openedEnv = SGF.createOpenedElementValueEnvironment({packComponentTy}, {&eltTy}, - {substExpansionType}, + {substComponentType}, {&substEltType}); - SGF.emitDynamicPackLoop(loc, inducedPackType, packComponentIndex, - openedEnv, - []() -> SILBasicBlock * { return nullptr; }, - [&](SILValue indexWithinComponent, - SILValue expansionPackIndex, - SILValue packIndex) { + auto inducedPackType = expansion.getFormalPackType(); + + SGF.emitPackForEach(loc, componentMV, inducedPackType, + packComponentIndex, openedEnv, eltTy, + [&](SILValue indexWithinComponent, + SILValue expansionPackIndex, + ManagedValue eltAddrMV) { componentInit->performPackExpansionInitialization(SGF, loc, indexWithinComponent, [&](Initialization *eltInit) { - // Project out the pack element and enter a managed value for it. - auto eltAddr = - SGF.B.createPackElementGet(loc, packIndex, packAddr, eltTy); - auto eltAddrMV = cloner.clone(eltAddr); - - auto result = handleScalar(eltAddrMV, origPatternType, substEltType, - eltInit, - /*inout*/ false, - /*addressable*/ false); - assert(result.isInContext()); (void) result; + auto eltResult = handleScalar(eltAddrMV, expansion.getOrigType(), + substEltType, eltInit, + /*inout*/ false, + /*addressable*/ false); + assert(eltResult.isInContext()); (void) eltResult; }); }); componentInit->finishInitialization(SGF); } + expansion.finish(); + + // If we emitted into an initialization, we're done. + if (init) { + // Make sure we finish the tuple initialization if we split it before. + if (!tupleVanishes) + init->finishInitialization(SGF); + + // If we created the initialization ourselves, return the managed + // address. + if (emitIntoTemporary) + return temporary->getManagedAddress(); + + // Otherwise, signal that we emitted into the provided intialization. + return ManagedValue::forInContext(); + } + + // Okay, there's no initialization, so we have to return a scalar value. + + // If the tuple pattern vanishes, the loop above should have produced a + // single "element". + if (tupleVanishes) { + assert(eltMVs.size() == 1); + return eltMVs[0]; + } + + // If the tuple doesn't vanish, we need to build the scalar element + // values into a scalar tuple values. + SmallVector eltValues; + eltValues.reserve(eltMVs.size()); + + // If all of the elements were guaranteed, we can form a guaranteed tuple. + if (tupleValueCanBeGuaranteed) { + for (auto element : eltMVs) { + eltValues.push_back(element.getUnmanagedValue()); + } + + // Otherwise, we need to move or copy values into a +1 tuple. + } else { + for (auto element : eltMVs) { + SILValue value = element.hasCleanup() + ? element.forward(SGF) + : element.copyUnmanaged(SGF, loc).forward(SGF); + eltValues.push_back(value); + } + } + auto tupleValue = SGF.B.createTuple(loc, substTL.getLoweredType(), eltValues); + if (tupleValue->getOwnershipKind() == OwnershipKind::None) + return ManagedValue::forObjectRValueWithoutOwnership(tupleValue); + return tupleValueCanBeGuaranteed + ? ManagedValue::forBorrowedObjectRValue(tupleValue) + : SGF.emitManagedRValueWithCleanup(tupleValue); } }; diff --git a/test/SILGen/let_decls.swift b/test/SILGen/let_decls.swift index e985acb24a4a1..5fff1f5238276 100644 --- a/test/SILGen/let_decls.swift +++ b/test/SILGen/let_decls.swift @@ -343,13 +343,13 @@ func testDebugValue(_ a : Int, b : SimpleProtocol) -> Int { // CHECK-LABEL: sil hidden [ossa] @{{.*}}testAddressOnlyTupleArgument func testAddressOnlyTupleArgument(_ bounds: (start: SimpleProtocol, pastEnd: Int)) { // CHECK: bb0(%0 : $*any SimpleProtocol, %1 : $Int): -// CHECK-NEXT: %2 = alloc_stack [lexical] [var_decl] $(start: any SimpleProtocol, pastEnd: Int), let, name "bounds", argno 1 -// CHECK-NEXT: %3 = tuple_element_addr %2 : $*(start: any SimpleProtocol, pastEnd: Int), 0 -// CHECK-NEXT: copy_addr %0 to [init] %3 : $*any SimpleProtocol -// CHECK-NEXT: %5 = tuple_element_addr %2 : $*(start: any SimpleProtocol, pastEnd: Int), 1 -// CHECK-NEXT: store %1 to [trivial] %5 : $*Int -// CHECK-NEXT: destroy_addr %2 : $*(start: any SimpleProtocol, pastEnd: Int) -// CHECK-NEXT: dealloc_stack %2 : $*(start: any SimpleProtocol, pastEnd: Int) +// CHECK-NEXT: [[TUPLE:%.*]] = alloc_stack [lexical] [var_decl] $(start: any SimpleProtocol, pastEnd: Int), let, name "bounds", argno 1 +// CHECK-NEXT: [[TUPLE_0:%.*]] = tuple_element_addr [[TUPLE]] : $*(start: any SimpleProtocol, pastEnd: Int), 0 +// CHECK-NEXT: [[TUPLE_1:%.*]] = tuple_element_addr [[TUPLE]] : $*(start: any SimpleProtocol, pastEnd: Int), 1 +// CHECK-NEXT: copy_addr %0 to [init] [[TUPLE_0]] : $*any SimpleProtocol +// CHECK-NEXT: store %1 to [trivial] [[TUPLE_1]] : $*Int +// CHECK-NEXT: destroy_addr [[TUPLE]] : $*(start: any SimpleProtocol, pastEnd: Int) +// CHECK-NEXT: dealloc_stack [[TUPLE]] : $*(start: any SimpleProtocol, pastEnd: Int) } diff --git a/test/SILGen/tuples.swift b/test/SILGen/tuples.swift index 23e1499ca847e..5b62bb2067f4a 100644 --- a/test/SILGen/tuples.swift +++ b/test/SILGen/tuples.swift @@ -148,9 +148,9 @@ extension P { // // Initialize the RValue. (This is here to help pattern matching). // CHECK: [[ZERO_ADDR:%.*]] = tuple_element_addr [[RVALUE]] : $*(index: C, value: Self), 0 + // CHECK: [[ONE_ADDR:%.*]] = tuple_element_addr [[RVALUE]] : $*(index: C, value: Self), 1 // CHECK: [[TUP0_COPY:%.*]] = copy_value [[TUP0]] // CHECK: store [[TUP0_COPY]] to [init] [[ZERO_ADDR]] - // CHECK: [[ONE_ADDR:%.*]] = tuple_element_addr [[RVALUE]] : $*(index: C, value: Self), 1 // CHECK: copy_addr [[TUP1]] to [init] [[ONE_ADDR]] // // What we are actually trying to check. Note that there is no actual use of diff --git a/test/SILGen/variadic-generic-vanishing-tuples.swift b/test/SILGen/variadic-generic-vanishing-tuples.swift index a42af1c6a36d2..0760609413c80 100644 --- a/test/SILGen/variadic-generic-vanishing-tuples.swift +++ b/test/SILGen/variadic-generic-vanishing-tuples.swift @@ -98,3 +98,34 @@ func makeTuple(_ t: repeat each T) -> (repeat each T) { public func makeOne(_ t: T) -> T { return makeTuple(t) } + +// https://github.com/swiftlang/swift/issues/69028, comment from June 11th, 2024 +// We were not handling vanishing tuples properly when binding parameters under +// an abstraction pattern. + +public func takeFunctionWithVariadicTupleParam(operation: @escaping ((repeat each Input)) -> Output) {} + +public func passSingleParamClosure() { + takeFunctionWithVariadicTupleParam(operation: { (num: Int) in num }) +} +// CHECK-LABEL: sil private [ossa] @$s4main22passSingleParamClosureyyFS2icfU_ : $@convention(thin) @substituted (@pack_guaranteed Pack{repeat each τ_0_0}) -> @out τ_0_1 for { +// CHECK: bb0(%0 : $*Int, %1 : $*Pack{Int}): +// CHECK-NEXT: [[PI:%.*]] = scalar_pack_index 0 of $Pack{Int} +// CHECK-NEXT: [[ARGPTR:%.*]] = pack_element_get [[PI]] of %1 +// CHECK-NEXT: [[ARG:%.*]] = load [trivial] [[ARGPTR]] +// CHECK-NEXT: debug_value [[ARG]] : $Int, let, name "num" +// CHECK-NEXT: store [[ARG]] to [trivial] %0 + +// Same thing but with non-trivial ownership +public func passSingleStringClosure() { + takeFunctionWithVariadicTupleParam(operation: { (str: String) in str }) +} +// CHECK-LABEL: sil private [ossa] @$s4main23passSingleStringClosureyyFS2ScfU_ : $@convention(thin) @substituted (@pack_guaranteed Pack{repeat each τ_0_0}) -> @out τ_0_1 for { +// CHECK: bb0(%0 : $*String, %1 : $*Pack{String}): +// CHECK-NEXT: [[PI:%.*]] = scalar_pack_index 0 of $Pack{String} +// CHECK-NEXT: [[ARGPTR:%.*]] = pack_element_get [[PI]] of %1 +// CHECK-NEXT: [[ARG:%.*]] = load_borrow [[ARGPTR]] +// CHECK-NEXT: debug_value [[ARG]] : $String, let, name "str" +// CHECK-NEXT: [[ARGCOPY:%.*]] = copy_value [[ARG]] +// CHECK-NEXT: store [[ARGCOPY]] to [init] %0 +// CHECK-NEXT: end_borrow [[ARG]] diff --git a/test/SILOptimizer/init_accessors.swift b/test/SILOptimizer/init_accessors.swift index d601cf7f2b032..8eca673dc29ee 100644 --- a/test/SILOptimizer/init_accessors.swift +++ b/test/SILOptimizer/init_accessors.swift @@ -335,17 +335,14 @@ struct TestGenericTuple { // // CHECK: bb0([[A_REF:%.*]] : $*T, [[B_REF:%.*]] : $*(T, U), [[A_VALUE:%.*]] : $*T, [[B_VALUE:%.*]] : $*T, [[C_VALUE:%.*]] : $*U, [[METATYPE:%.*]] : $@thin TestGenericTuple.Type): // - // CHECK: [[INIT_VALUE_1:%.*]] = alloc_stack $(T, U) - // CHECK-NEXT: [[INIT_VALUE_1_0:%.*]] = tuple_element_addr [[INIT_VALUE_1]] : $*(T, U), 0 - // CHECK-NEXT: copy_addr [take] [[B_VALUE]] to [init] [[INIT_VALUE_1_0]] - // CHECK-NEXT: [[INIT_VALUE_1_1:%.*]] = tuple_element_addr [[INIT_VALUE_1]] : $*(T, U), 1 - // CHECK-NEXT: copy_addr [take] [[C_VALUE]] to [init] [[INIT_VALUE_1_1]] - - // CHECK-NEXT: [[INIT_VALUE_2:%.*]] = alloc_stack [lexical] [var_decl] $(T, (T, U)) - // CHECK-NEXT: [[INIT_VALUE_2_0:%.*]] = tuple_element_addr [[INIT_VALUE_2]] : $*(T, (T, U)), 0 - // CHECK-NEXT: copy_addr [take] [[A_VALUE]] to [init] [[INIT_VALUE_2_0]] - // CHECK-NEXT: [[INIT_VALUE_2_1:%.*]] = tuple_element_addr [[INIT_VALUE_2]] : $*(T, (T, U)), 1 - // CHECK-NEXT: copy_addr [take] [[INIT_VALUE_1]] to [init] [[INIT_VALUE_2_1]] + // CHECK: [[TUPLE:%.*]] = alloc_stack [lexical] [var_decl] $(T, (T, U)) + // CHECK-NEXT: [[TUPLE_0:%.*]] = tuple_element_addr [[TUPLE]] : $*(T, (T, U)), 0 + // CHECK-NEXT: [[TUPLE_1:%.*]] = tuple_element_addr [[TUPLE]] : $*(T, (T, U)), 1 + // CHECK-NEXT: copy_addr [take] [[A_VALUE]] to [init] [[TUPLE_0]] + // CHECK-NEXT: [[TUPLE_1_0:%.*]] = tuple_element_addr [[TUPLE_1]] : $*(T, U), 0 + // CHECK-NEXT: [[TUPLE_1_1:%.*]] = tuple_element_addr [[TUPLE_1]] : $*(T, U), 1 + // CHECK-NEXT: copy_addr [take] [[B_VALUE]] to [init] [[TUPLE_1_0]] + // CHECK-NEXT: copy_addr [take] [[C_VALUE]] to [init] [[TUPLE_1_1]] var data: (T, (T, U)) { @storageRestrictions(initializes: a, b)