From 280c91a54f4b882326e4c24aca4fa0927076d824 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 27 Mar 2023 17:15:24 -0400 Subject: [PATCH 1/5] Fix the dumping of AbstractionPatterns with substitutions --- lib/SIL/IR/AbstractionPattern.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/SIL/IR/AbstractionPattern.cpp b/lib/SIL/IR/AbstractionPattern.cpp index 6f8e375959366..8ab7eb4790691 100644 --- a/lib/SIL/IR/AbstractionPattern.cpp +++ b/lib/SIL/IR/AbstractionPattern.cpp @@ -1373,12 +1373,12 @@ static void printGenerics(raw_ostream &out, const AbstractionPattern &pattern) { // It'd be really nice if we could get these interleaved with the types. if (auto subs = pattern.getGenericSubstitutions()) { out << "@<"; - bool first = false; + bool first = true; for (auto sub : subs.getReplacementTypes()) { if (!first) { out << ","; } else { - first = true; + first = false; } out << sub; } From 8421669a5927ea77abc594b6d9976daa1a11bd63 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 27 Mar 2023 17:15:57 -0400 Subject: [PATCH 2/5] Pass down the substitutions of the original pattern when extracting a subst abstraction pattern from a generic nominal type. --- lib/SIL/IR/AbstractionPattern.cpp | 19 +++++++++++-------- test/SILGen/variadic-generic-closures.swift | 6 ++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/SIL/IR/AbstractionPattern.cpp b/lib/SIL/IR/AbstractionPattern.cpp index 8ab7eb4790691..931da4a9a8d9c 100644 --- a/lib/SIL/IR/AbstractionPattern.cpp +++ b/lib/SIL/IR/AbstractionPattern.cpp @@ -1961,8 +1961,11 @@ class SubstFunctionTypePatternVisitor : CanType(CanMetatypeType::get(substInstance)); } - CanType handleGenericNominalType(CanType orig, CanType subst, - CanGenericSignature origSig) { + CanType handleGenericNominalType(AbstractionPattern origPattern, CanType subst) { + CanType orig = origPattern.getType(); + CanGenericSignature origSig = origPattern.getGenericSignatureOrNull(); + auto origPatternSubs = origPattern.getGenericSubstitutions(); + // If there are no loose type parameters in the pattern here, we don't need // to do a recursive visit at all. if (!orig->hasTypeParameter() @@ -2007,6 +2010,7 @@ class SubstFunctionTypePatternVisitor if (differentOrigClass) { orig = subst; origSig = TC.getCurGenericSignature(); + origPatternSubs = SubstitutionMap(); assert((!subst->hasTypeParameter() || origSig) && "lowering mismatched interface types in a context without " "a generic signature"); @@ -2028,7 +2032,8 @@ class SubstFunctionTypePatternVisitor ->getCanonicalType(); replacementTypes[gp->getCanonicalType()->castTo()] - = visit(substParamTy, AbstractionPattern(origSig, origParamTy)); + = visit(substParamTy, + AbstractionPattern(origPatternSubs, origSig, origParamTy)); } auto newSubMap = SubstitutionMap::get(nomGenericSig, @@ -2048,8 +2053,7 @@ class SubstFunctionTypePatternVisitor // If the type is generic (because it's a nested type in a generic context), // process the generic type bindings. if (!isa(nomDecl) && nomDecl->isGenericContext()) { - return handleGenericNominalType(pattern.getType(), nom, - pattern.getGenericSignatureOrNull()); + return handleGenericNominalType(pattern, nom); } // Otherwise, there are no structural type parameters to visit. @@ -2058,8 +2062,7 @@ class SubstFunctionTypePatternVisitor CanType visitBoundGenericType(CanBoundGenericType bgt, AbstractionPattern pattern) { - return handleGenericNominalType(pattern.getType(), bgt, - pattern.getGenericSignatureOrNull()); + return handleGenericNominalType(pattern, bgt); } CanType visitPackType(CanPackType pack, AbstractionPattern pattern) { @@ -2085,7 +2088,7 @@ class SubstFunctionTypePatternVisitor // the pack substitution for that parameter recorded in the pattern. // Remember that we're within an expansion. - // FIXME: when we introduce PackReferenceType we'll need to be clear + // FIXME: when we introduce PackElementType we'll need to be clear // about which pack expansions to treat this way. llvm::SaveAndRestore scope(WithinExpansion, true); diff --git a/test/SILGen/variadic-generic-closures.swift b/test/SILGen/variadic-generic-closures.swift index b9a026f4f1573..16cbb6c847b25 100644 --- a/test/SILGen/variadic-generic-closures.swift +++ b/test/SILGen/variadic-generic-closures.swift @@ -9,3 +9,9 @@ public struct G {} public func caller(fn: (repeat G) -> ()) { fn(repeat G()) } + +// rdar://107108803 +public struct UsesG { + public init(builder: (repeat G) -> E) {} +} +UsesG { a, b, c in 0 } From 08a921198a2bc940daae137f83d4f0a0277e363d Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 27 Mar 2023 17:17:18 -0400 Subject: [PATCH 3/5] Assert that we don't produce a SILFunction with a type with opened type parameters. --- lib/SIL/IR/SILFunction.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/SIL/IR/SILFunction.cpp b/lib/SIL/IR/SILFunction.cpp index f3107f50a95e6..eca0c14b3e613 100644 --- a/lib/SIL/IR/SILFunction.cpp +++ b/lib/SIL/IR/SILFunction.cpp @@ -186,6 +186,10 @@ void SILFunction::init( IsExactSelfClass_t isExactSelfClass, IsDistributed_t isDistributed, IsRuntimeAccessible_t isRuntimeAccessible) { setName(Name); + + assert(!LoweredType->hasTypeParameter() && + "function type has open type parameters"); + this->LoweredType = LoweredType; this->GenericEnv = genericEnv; this->SpecializationInfo = nullptr; From de283f8053afe4cd9a6cbb7aa7c24e1dddc744f2 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 27 Mar 2023 17:23:59 -0400 Subject: [PATCH 4/5] Make BlackHoleInitialization support pack initialization Fixes rdar://107151145 --- lib/SILGen/Initialization.h | 9 +++++++++ lib/SILGen/SILGenDecl.cpp | 9 +++++++++ test/SILGen/variadic-generic-tuples.swift | 8 ++++++++ 3 files changed, 26 insertions(+) diff --git a/lib/SILGen/Initialization.h b/lib/SILGen/Initialization.h index 1bced77f1e065..93081d4970c56 100644 --- a/lib/SILGen/Initialization.h +++ b/lib/SILGen/Initialization.h @@ -491,6 +491,15 @@ class BlackHoleInitialization : public Initialization { return buf; } + bool canPerformPackExpansionInitialization() const override { + return true; + } + + void performPackExpansionInitialization(SILGenFunction &SGF, + SILLocation loc, + SILValue indexWithinComponent, + llvm::function_ref fn) override; + void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc, ManagedValue value, bool isInit) override; diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp index 6ead74f108574..9db9e9ac92a6e 100644 --- a/lib/SILGen/SILGenDecl.cpp +++ b/lib/SILGen/SILGenDecl.cpp @@ -2172,6 +2172,15 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) { llvm_unreachable("unhandled case"); } +void BlackHoleInitialization::performPackExpansionInitialization( + SILGenFunction &SGF, + SILLocation loc, + SILValue indexWithinComponent, + llvm::function_ref fn) { + BlackHoleInitialization subInit; + fn(&subInit); +} + void BlackHoleInitialization::copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc, ManagedValue value, bool isInit) { // Normally we do not do anything if we have a black hole diff --git a/test/SILGen/variadic-generic-tuples.swift b/test/SILGen/variadic-generic-tuples.swift index b1585c5804b98..bde4743a3762d 100644 --- a/test/SILGen/variadic-generic-tuples.swift +++ b/test/SILGen/variadic-generic-tuples.swift @@ -283,3 +283,11 @@ struct MemberwiseTupleHolder { func callVariadicMemberwiseInit() -> MemberwiseTupleHolder { return MemberwiseTupleHolder(content: (0, "hello")) } + +// rdar://107151145: when we tuple-destructure a black hole +// initialization, the resulting element initializations need to +// handle pack expansion initialization +struct EmptyContainer {} +func f(_: repeat each T) { + let _ = (repeat EmptyContainer()) +} From 74420994d47f8a7624afd1bef84dfd5abd757694 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 28 Mar 2023 14:32:58 -0400 Subject: [PATCH 5/5] Teach ResultPlan to handle packs correctly when we're not emitting into an Initialization. rdar://107161241 --- lib/SILGen/ResultPlan.cpp | 135 ++++++++++++++-------- lib/SILGen/ResultPlan.h | 10 +- test/SILGen/variadic-generic-tuples.swift | 32 +++++ 3 files changed, 128 insertions(+), 49 deletions(-) diff --git a/lib/SILGen/ResultPlan.cpp b/lib/SILGen/ResultPlan.cpp index 99875cfc01f7c..979ce327e82bf 100644 --- a/lib/SILGen/ResultPlan.cpp +++ b/lib/SILGen/ResultPlan.cpp @@ -27,6 +27,15 @@ using namespace Lowering; // Result Plans //===----------------------------------------------------------------------===// +void ResultPlan::finishAndAddTo(SILGenFunction &SGF, SILLocation loc, + ArrayRef &directResults, + SILValue bridgedForeignError, + RValue &result) { + auto rvalue = finish(SGF, loc, directResults, bridgedForeignError); + assert(!rvalue.isInContext()); + result.addElement(std::move(rvalue)); +} + namespace { /// A result plan for evaluating an indirect result into the address @@ -343,14 +352,16 @@ class ScalarResultPlan final : public ResultPlan { /// using a temporary buffer initialized by a sub-plan. class InitValueFromTemporaryResultPlan final : public ResultPlan { Initialization *init; + CanType substType; ResultPlanPtr subPlan; std::unique_ptr temporary; public: InitValueFromTemporaryResultPlan( - Initialization *init, ResultPlanPtr &&subPlan, + Initialization *init, CanType substType, + ResultPlanPtr &&subPlan, std::unique_ptr &&temporary) - : init(init), subPlan(std::move(subPlan)), + : init(init), substType(substType), subPlan(std::move(subPlan)), temporary(std::move(temporary)) {} RValue finish(SILGenFunction &SGF, SILLocation loc, @@ -362,10 +373,15 @@ class InitValueFromTemporaryResultPlan final : public ResultPlan { (void)subResult; ManagedValue value = temporary->getManagedAddress(); - init->copyOrInitValueInto(SGF, loc, value, /*init*/ true); - init->finishInitialization(SGF); - return RValue::forInContext(); + if (init) { + init->copyOrInitValueInto(SGF, loc, value, /*init*/ true); + init->finishInitialization(SGF); + + return RValue::forInContext(); + } + + return RValue(SGF, loc, substType, value); } void @@ -414,28 +430,30 @@ class PackExpansionResultPlan : public ResultPlan { public: PackExpansionResultPlan(ResultPlanBuilder &builder, SILValue packAddr, - MutableArrayRef inits, + Optional> inits, AbstractionPattern origExpansionType, CanTupleEltTypeArrayRef substEltTypes) : PackAddr(packAddr) { + assert(!inits || inits->size() == substEltTypes.size()); + auto packTy = packAddr->getType().castTo(); auto formalPackType = CanPackType::get(packTy->getASTContext(), substEltTypes); auto origPatternType = origExpansionType.getPackExpansionPatternType(); - ComponentPlans.reserve(inits.size()); - for (auto i : indices(inits)) { - auto &init = inits[i]; + ComponentPlans.reserve(substEltTypes.size()); + for (auto i : indices(substEltTypes)) { + Initialization *init = inits ? (*inits)[i].get() : nullptr; CanType substEltType = substEltTypes[i]; if (isa(substEltType)) { ComponentPlans.emplace_back( builder.buildPackExpansionIntoPack(packAddr, formalPackType, i, - init.get(), origPatternType)); + init, origPatternType)); } else { ComponentPlans.emplace_back( builder.buildScalarIntoPack(packAddr, formalPackType, i, - init.get(), origPatternType)); + init, origPatternType)); } } } @@ -451,6 +469,16 @@ class PackExpansionResultPlan : public ResultPlan { return RValue::forInContext(); } + void finishAndAddTo(SILGenFunction &SGF, SILLocation loc, + ArrayRef &directResults, + SILValue bridgedForeignError, + RValue &result) override { + for (auto &componentPlan : ComponentPlans) { + componentPlan->finishAndAddTo(SGF, loc, directResults, + bridgedForeignError, result); + } + } + void gatherIndirectResultAddrs(SILGenFunction &SGF, SILLocation loc, SmallVectorImpl &outList) const override { outList.push_back(PackAddr); @@ -557,19 +585,27 @@ class PackTransformResultPlan final : public ResultPlan { /// components. class TupleRValueResultPlan final : public ResultPlan { CanTupleType substType; - SmallVector eltPlans; + + SmallVector origEltPlans; public: TupleRValueResultPlan(ResultPlanBuilder &builder, AbstractionPattern origType, CanTupleType substType) : substType(substType) { // Create plans for all the elements. - eltPlans.reserve(substType->getNumElements()); - for (auto i : indices(substType->getElementTypes())) { - AbstractionPattern origEltType = origType.getTupleElementType(i); - CanType substEltType = substType.getElementType(i); - eltPlans.push_back(builder.build(nullptr, origEltType, substEltType)); - } + origEltPlans.reserve(substType->getNumElements()); + origType.forEachTupleElement(substType, + [&](TupleElementGenerator &origElt) { + AbstractionPattern origEltType = origElt.getOrigType(); + auto substEltTypes = origElt.getSubstTypes(); + if (!origElt.isOrigPackExpansion()) { + origEltPlans.push_back( + builder.build(nullptr, origEltType, substEltTypes[0])); + } else { + origEltPlans.push_back( + builder.buildForPackExpansion(None, origEltType, substEltTypes)); + } + }); } RValue finish(SILGenFunction &SGF, SILLocation loc, @@ -578,10 +614,9 @@ class TupleRValueResultPlan final : public ResultPlan { RValue tupleRV(substType); // Finish all the component tuples. - for (auto &plan : eltPlans) { - RValue eltRV = - plan->finish(SGF, loc, directResults, bridgedForeignError); - tupleRV.addElement(std::move(eltRV)); + for (auto &plan : origEltPlans) { + plan->finishAndAddTo(SGF, loc, directResults, bridgedForeignError, + tupleRV); } return tupleRV; @@ -590,8 +625,8 @@ class TupleRValueResultPlan final : public ResultPlan { void gatherIndirectResultAddrs(SILGenFunction &SGF, SILLocation loc, SmallVectorImpl &outList) const override { - for (const auto &eltPlan : eltPlans) { - eltPlan->gatherIndirectResultAddrs(SGF, loc, outList); + for (const auto &plan : origEltPlans) { + plan->gatherIndirectResultAddrs(SGF, loc, outList); } } }; @@ -1143,10 +1178,10 @@ ResultPlanPtr ResultPlanBuilder::buildForScalar(Initialization *init, } ResultPlanPtr ResultPlanBuilder:: - buildForPackExpansion(MutableArrayRef inits, + buildForPackExpansion(Optional> inits, AbstractionPattern origExpansionType, CanTupleEltTypeArrayRef substTypes) { - assert(inits.size() == substTypes.size()); + assert(!inits || inits->size() == substTypes.size()); // Pack expansions in the original result type always turn into // a single @pack_out result. @@ -1155,7 +1190,7 @@ ResultPlanPtr ResultPlanBuilder:: auto packTy = result.getSILStorageType(SGF.SGM.M, calleeTypeInfo.substFnType, SGF.getTypeExpansionContext()); - assert(packTy.castTo()->getNumElements() == inits.size()); + assert(packTy.castTo()->getNumElements() == substTypes.size()); // TODO: try to just forward a single pack @@ -1236,7 +1271,6 @@ ResultPlanBuilder::buildScalarIntoPack(SILValue packAddr, Initialization *init, AbstractionPattern origType) { assert(!origType.isPackExpansion()); - assert(init); auto substType = formalPackType.getElementType(componentIndex); assert(!isa(substType)); @@ -1245,7 +1279,8 @@ ResultPlanBuilder::buildScalarIntoPack(SILValue packAddr, ->getElementType(componentIndex); SILResultInfo resultInfo(loweredEltType, ResultConvention::Indirect); - // Use the normal scalar emission path. + // Use the normal scalar emission path to gather an indirect result + // of that type. auto plan = buildForScalar(init, origType, substType, resultInfo); // Immediately gather the indirect result. @@ -1265,38 +1300,44 @@ ResultPlanBuilder::buildScalarIntoPack(SILValue packAddr, ResultPlanPtr ResultPlanBuilder::buildForTuple(Initialization *init, AbstractionPattern origType, CanTupleType substType) { - // If we don't have an initialization for the tuple, just build the - // individual components. - if (!init) { - return ResultPlanPtr(new TupleRValueResultPlan(*this, origType, substType)); - } - - // Okay, we have an initialization for the tuple that we need to emit into. - - // If we can just split the initialization, do so. - if (init->canSplitIntoTupleElements()) { + // If we have an initialization, and we can split it, do so. + if (init && init->canSplitIntoTupleElements()) { return ResultPlanPtr( new TupleInitializationResultPlan(*this, init, origType, substType)); } - // Otherwise, we're going to have to call copyOrInitValueInto, which only - // takes a single value. - - // If the tuple is address-only, we'll get much better code if we - // emit into a single buffer. + // Otherwise, if the tuple contains a pack expansion, we'll need to + // initialize a single buffer one way or another: either we're giving + // this to RValue (which wants a single value for tuples with pack + // expansions) or we'll have to call copyOrInitValueInto on init + // (which expects a single value). Create a temporary, build into + // that, and then call the initialization. + // + // We also use this path when we have an init and the type is + // address-only, because we'll need to call copyOrInitValueInto and + // we'll get better code by building that up indirectly. But we don't + // do that if we're not using lowered addresses because we prefer to + // build tuples with scalar operations. auto &substTL = SGF.getTypeLowering(substType); + assert(substTL.isAddressOnly() || !substType.containsPackExpansionType()); if (substTL.isAddressOnly() && (substType.containsPackExpansionType() || - SGF.F.getConventions().useLoweredAddresses())) { + (init != nullptr && SGF.F.getConventions().useLoweredAddresses()))) { // Create a temporary. auto temporary = SGF.emitTemporary(loc, substTL); // Build a sub-plan to emit into the temporary. auto subplan = buildForTuple(temporary.get(), origType, substType); - // Make a plan to initialize into that. + // Make a plan to produce the final result from that. return ResultPlanPtr(new InitValueFromTemporaryResultPlan( - init, std::move(subplan), std::move(temporary))); + init, substType, std::move(subplan), std::move(temporary))); + } + + // If we don't have an initialization, just build the individual + // components. + if (!init) { + return ResultPlanPtr(new TupleRValueResultPlan(*this, origType, substType)); } // Build a sub-plan that doesn't know about the initialization. diff --git a/lib/SILGen/ResultPlan.h b/lib/SILGen/ResultPlan.h index 3322cc8ba1cc1..f194fdfc86625 100644 --- a/lib/SILGen/ResultPlan.h +++ b/lib/SILGen/ResultPlan.h @@ -42,6 +42,12 @@ class ResultPlan { virtual RValue finish(SILGenFunction &SGF, SILLocation loc, ArrayRef &directResults, SILValue bridgedForeignError) = 0; + + virtual void finishAndAddTo(SILGenFunction &SGF, SILLocation loc, + ArrayRef &directResults, + SILValue bridgedForeignError, + RValue &result); + virtual ~ResultPlan() = default; /// Defers the emission of the given breadcrumb until \p finish is invoked. @@ -92,8 +98,8 @@ struct ResultPlanBuilder { ResultPlanPtr buildForTuple(Initialization *emitInto, AbstractionPattern origType, CanTupleType substType); - ResultPlanPtr buildForPackExpansion(MutableArrayRef inits, - AbstractionPattern origPatternType, + ResultPlanPtr buildForPackExpansion(Optional> inits, + AbstractionPattern origExpansionType, CanTupleEltTypeArrayRef substTypes); ResultPlanPtr buildPackExpansionIntoPack(SILValue packAddr, CanPackType formalPackType, diff --git a/test/SILGen/variadic-generic-tuples.swift b/test/SILGen/variadic-generic-tuples.swift index bde4743a3762d..d86ebecb0e395 100644 --- a/test/SILGen/variadic-generic-tuples.swift +++ b/test/SILGen/variadic-generic-tuples.swift @@ -291,3 +291,35 @@ struct EmptyContainer {} func f(_: repeat each T) { let _ = (repeat EmptyContainer()) } + +// rdar://107161241: handle receiving tuples that originally contained +// packs that are not emitted into an initialization +struct FancyTuple { + var x: (repeat each T) + + func makeTuple() -> (repeat each T) { + return (repeat each x.element) + } +} + +// CHECK: sil{{.*}} @$s4main23testFancyTuple_concreteyyF : +// Create a pack to receive the results from makeTuple. +// CHECK: [[PACK:%.*]] = alloc_pack $Pack{Int, String, Bool} +// CHECK-NEXT: [[INT_ADDR:%.*]] = alloc_stack $Int +// CHECK-NEXT: [[INT_INDEX:%.*]] = scalar_pack_index 0 of $Pack{Int, String, Bool} +// CHECK-NEXT: pack_element_set [[INT_ADDR]] : $*Int into [[INT_INDEX]] of [[PACK]] : $*Pack{Int, String, Bool} +// CHECK-NEXT: [[STRING_ADDR:%.*]] = alloc_stack $String +// CHECK-NEXT: [[STRING_INDEX:%.*]] = scalar_pack_index 1 of $Pack{Int, String, Bool} +// CHECK-NEXT: pack_element_set [[STRING_ADDR]] : $*String into [[STRING_INDEX]] of [[PACK]] : $*Pack{Int, String, Bool} +// CHECK-NEXT: [[BOOL_ADDR:%.*]] = alloc_stack $Bool +// CHECK-NEXT: [[BOOL_INDEX:%.*]] = scalar_pack_index 2 of $Pack{Int, String, Bool} +// CHECK-NEXT: pack_element_set [[BOOL_ADDR]] : $*Bool into [[BOOL_INDEX]] of [[PACK]] : $*Pack{Int, String, Bool} +// CHECK: [[FN:%.*]] = function_ref @$s4main10FancyTupleV04makeC0xxQp_tyF +// CHECK-NEXT: apply [[FN]]([[PACK]], {{.*}}) +func testFancyTuple_concrete() { + FancyTuple(x: (1, "hi", false)).makeTuple() +} + +func testFancyTuple_pack(values: repeat each T) { + FancyTuple(x: (1, "hi", repeat each values, false)).makeTuple() +}