Skip to content

Commit 1ead872

Browse files
rjmccallrjmccall
authored
Merge pull request #64293 from rjmccall/more-variadic-fixes
Fix some bugs with variadic tuple lowering and value-op outlining
2 parents ee0e38a + 81c4e07 commit 1ead872

File tree

12 files changed

+169
-50
lines changed

12 files changed

+169
-50
lines changed

‎include/swift/SIL/SILType.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,12 @@ class SILType {
405405
bool hasOpenedExistential() const {
406406
return getASTType()->hasOpenedExistential();
407407
}
408+
409+
/// Returns true if the referenced type is expressed in terms of one
410+
/// or more local archetypes.
411+
bool hasLocalArchetype() const {
412+
return getASTType()->hasLocalArchetype();
413+
}
408414

409415
/// Returns the representation used by an existential type. If the concrete
410416
/// type is provided, this may return a specialized representation kind that

‎lib/IRGen/GenEnum.cpp

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ namespace {
549549
if (!getSingleton()) return;
550550
if (!ElementsAreABIAccessible) {
551551
emitAssignWithCopyCall(IGF, T, dest, src);
552-
} else if (isOutlined || T.hasOpenedExistential()) {
552+
} else if (isOutlined || T.hasLocalArchetype()) {
553553
dest = getSingletonAddress(IGF, dest);
554554
src = getSingletonAddress(IGF, src);
555555
getSingleton()->assignWithCopy(
@@ -564,7 +564,7 @@ namespace {
564564
if (!getSingleton()) return;
565565
if (!ElementsAreABIAccessible) {
566566
emitAssignWithTakeCall(IGF, T, dest, src);
567-
} else if (isOutlined || T.hasOpenedExistential()) {
567+
} else if (isOutlined || T.hasLocalArchetype()) {
568568
dest = getSingletonAddress(IGF, dest);
569569
src = getSingletonAddress(IGF, src);
570570
getSingleton()->assignWithTake(
@@ -586,7 +586,7 @@ namespace {
586586
if (!getSingleton()) return;
587587
if (!ElementsAreABIAccessible) {
588588
emitInitializeWithCopyCall(IGF, T, dest, src);
589-
} else if (isOutlined || T.hasOpenedExistential()) {
589+
} else if (isOutlined || T.hasLocalArchetype()) {
590590
dest = getSingletonAddress(IGF, dest);
591591
src = getSingletonAddress(IGF, src);
592592
getSingleton()->initializeWithCopy(
@@ -601,7 +601,7 @@ namespace {
601601
if (!getSingleton()) return;
602602
if (!ElementsAreABIAccessible) {
603603
emitInitializeWithTakeCall(IGF, T, dest, src);
604-
} else if (isOutlined || T.hasOpenedExistential()) {
604+
} else if (isOutlined || T.hasLocalArchetype()) {
605605
dest = getSingletonAddress(IGF, dest);
606606
src = getSingletonAddress(IGF, src);
607607
getSingleton()->initializeWithTake(
@@ -657,7 +657,7 @@ namespace {
657657
!getSingleton()->isTriviallyDestroyable(ResilienceExpansion::Maximal)) {
658658
if (!ElementsAreABIAccessible) {
659659
emitDestroyCall(IGF, T, addr);
660-
} else if (isOutlined || T.hasOpenedExistential()) {
660+
} else if (isOutlined || T.hasLocalArchetype()) {
661661
getSingleton()->destroy(IGF, getSingletonAddress(IGF, addr),
662662
getSingletonType(IGF.IGM, T), isOutlined);
663663
} else {
@@ -2615,7 +2615,7 @@ namespace {
26152615
llvm_unreachable("ABI-inaccessible type cannot be loadable");
26162616

26172617
case Normal: {
2618-
if (loweredType.hasOpenedExistential()) {
2618+
if (loweredType.hasLocalArchetype()) {
26192619
EnumPayload payload;
26202620
llvm::Value *extraTag;
26212621
std::tie(payload, extraTag) =
@@ -2688,7 +2688,7 @@ namespace {
26882688
llvm_unreachable("ABI-inaccessible type cannot be loadable");
26892689

26902690
case Normal: {
2691-
if (loweredType.hasOpenedExistential()) {
2691+
if (loweredType.hasLocalArchetype()) {
26922692
EnumPayload payload;
26932693
llvm::Value *extraTag;
26942694
std::tie(payload, extraTag) =
@@ -2810,7 +2810,7 @@ namespace {
28102810
}
28112811
if (!ElementsAreABIAccessible) {
28122812
return emitDestroyCall(IGF, T, addr);
2813-
} else if (isOutlined || T.hasOpenedExistential()) {
2813+
} else if (isOutlined || T.hasLocalArchetype()) {
28142814
switch (CopyDestroyKind) {
28152815
case TriviallyDestroyable:
28162816
return;
@@ -3114,7 +3114,7 @@ namespace {
31143114
SILType T, bool isOutlined) const override {
31153115
if (!ElementsAreABIAccessible) {
31163116
emitAssignWithCopyCall(IGF, T, dest, src);
3117-
} else if (isOutlined || T.hasOpenedExistential()) {
3117+
} else if (isOutlined || T.hasLocalArchetype()) {
31183118
emitIndirectAssign(IGF, dest, src, T, IsNotTake, isOutlined);
31193119
} else {
31203120
callOutlinedCopy(IGF, dest, src, T, IsNotInitialization, IsNotTake);
@@ -3125,7 +3125,7 @@ namespace {
31253125
SILType T, bool isOutlined) const override {
31263126
if (!ElementsAreABIAccessible) {
31273127
emitAssignWithTakeCall(IGF, T, dest, src);
3128-
} else if (isOutlined || T.hasOpenedExistential()) {
3128+
} else if (isOutlined || T.hasLocalArchetype()) {
31293129
emitIndirectAssign(IGF, dest, src, T, IsTake, isOutlined);
31303130
} else {
31313131
callOutlinedCopy(IGF, dest, src, T, IsNotInitialization, IsTake);
@@ -3136,7 +3136,7 @@ namespace {
31363136
SILType T, bool isOutlined) const override {
31373137
if (!ElementsAreABIAccessible) {
31383138
emitInitializeWithCopyCall(IGF, T, dest, src);
3139-
} else if (isOutlined || T.hasOpenedExistential()) {
3139+
} else if (isOutlined || T.hasLocalArchetype()) {
31403140
emitIndirectInitialize(IGF, dest, src, T, IsNotTake, isOutlined);
31413141
} else {
31423142
callOutlinedCopy(IGF, dest, src, T, IsInitialization, IsNotTake);
@@ -3147,7 +3147,7 @@ namespace {
31473147
SILType T, bool isOutlined) const override {
31483148
if (!ElementsAreABIAccessible) {
31493149
emitInitializeWithTakeCall(IGF, T, dest, src);
3150-
} else if (isOutlined || T.hasOpenedExistential()) {
3150+
} else if (isOutlined || T.hasLocalArchetype()) {
31513151
emitIndirectInitialize(IGF, dest, src, T, IsTake, isOutlined);
31523152
} else {
31533153
callOutlinedCopy(IGF, dest, src, T, IsInitialization, IsTake);
@@ -4633,7 +4633,7 @@ namespace {
46334633

46344634
case BitwiseTakable:
46354635
case Normal: {
4636-
if (loweredType.hasOpenedExistential()) {
4636+
if (loweredType.hasLocalArchetype()) {
46374637
auto parts = destructureAndTagLoadableEnum(IGF, src);
46384638

46394639
forNontrivialPayloads(
@@ -4698,7 +4698,7 @@ namespace {
46984698

46994699
case BitwiseTakable:
47004700
case Normal: {
4701-
if (loweredType.hasOpenedExistential()) {
4701+
if (loweredType.hasLocalArchetype()) {
47024702
auto parts = destructureAndTagLoadableEnum(IGF, src);
47034703

47044704
forNontrivialPayloads(
@@ -4972,7 +4972,7 @@ namespace {
49724972
SILType T, bool isOutlined) const override {
49734973
if (!ElementsAreABIAccessible) {
49744974
emitAssignWithCopyCall(IGF, T, dest, src);
4975-
} else if (isOutlined || T.hasOpenedExistential()) {
4975+
} else if (isOutlined || T.hasLocalArchetype()) {
49764976
emitIndirectAssign(IGF, dest, src, T, IsNotTake, isOutlined);
49774977
} else {
49784978
callOutlinedCopy(IGF, dest, src, T, IsNotInitialization, IsNotTake);
@@ -4983,7 +4983,7 @@ namespace {
49834983
SILType T, bool isOutlined) const override {
49844984
if (!ElementsAreABIAccessible) {
49854985
emitAssignWithTakeCall(IGF, T, dest, src);
4986-
} else if (isOutlined || T.hasOpenedExistential()) {
4986+
} else if (isOutlined || T.hasLocalArchetype()) {
49874987
emitIndirectAssign(IGF, dest, src, T, IsTake, isOutlined);
49884988
} else {
49894989
callOutlinedCopy(IGF, dest, src, T, IsNotInitialization, IsTake);
@@ -4994,7 +4994,7 @@ namespace {
49944994
SILType T, bool isOutlined) const override {
49954995
if (!ElementsAreABIAccessible) {
49964996
emitInitializeWithCopyCall(IGF, T, dest, src);
4997-
} else if (isOutlined || T.hasOpenedExistential()) {
4997+
} else if (isOutlined || T.hasLocalArchetype()) {
49984998
emitIndirectInitialize(IGF, dest, src, T, IsNotTake, isOutlined);
49994999
} else {
50005000
callOutlinedCopy(IGF, dest, src, T, IsInitialization, IsNotTake);
@@ -5005,7 +5005,7 @@ namespace {
50055005
SILType T, bool isOutlined) const override {
50065006
if (!ElementsAreABIAccessible) {
50075007
emitInitializeWithTakeCall(IGF, T, dest, src);
5008-
} else if (isOutlined || T.hasOpenedExistential()) {
5008+
} else if (isOutlined || T.hasLocalArchetype()) {
50095009
emitIndirectInitialize(IGF, dest, src, T, IsTake, isOutlined);
50105010
} else {
50115011
callOutlinedCopy(IGF, dest, src, T, IsInitialization, IsTake);
@@ -5039,7 +5039,7 @@ namespace {
50395039
}
50405040
if (!ElementsAreABIAccessible) {
50415041
emitDestroyCall(IGF, T, addr);
5042-
} else if (isOutlined || T.hasOpenedExistential()) {
5042+
} else if (isOutlined || T.hasLocalArchetype()) {
50435043
switch (CopyDestroyKind) {
50445044
case TriviallyDestroyable:
50455045
return;

‎lib/IRGen/GenProto.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -765,8 +765,8 @@ static bool shouldSetName(IRGenModule &IGM, llvm::Value *value, CanType type) {
765765
// If value names are globally disabled, honor that.
766766
if (!IGM.EnableValueNames) return false;
767767

768-
// Suppress value names for values with opened existentials.
769-
if (type->hasOpenedExistential()) return false;
768+
// Suppress value names for values with local archetypes
769+
if (type->hasLocalArchetype()) return false;
770770

771771
// If the value already has a name, honor that.
772772
if (value->hasName()) return false;

‎lib/IRGen/GenRecord.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class RecordTypeInfoImpl : public Base,
170170
return emitAssignWithCopyCall(IGF, T, dest, src);
171171
}
172172

173-
if (isOutlined || T.hasOpenedExistential()) {
173+
if (isOutlined || T.hasLocalArchetype()) {
174174
auto offsets = asImpl().getNonFixedOffsets(IGF, T);
175175
for (auto &field : getFields()) {
176176
if (field.isEmpty())
@@ -193,7 +193,7 @@ class RecordTypeInfoImpl : public Base,
193193
return emitAssignWithTakeCall(IGF, T, dest, src);
194194
}
195195

196-
if (isOutlined || T.hasOpenedExistential()) {
196+
if (isOutlined || T.hasLocalArchetype()) {
197197
auto offsets = asImpl().getNonFixedOffsets(IGF, T);
198198
for (auto &field : getFields()) {
199199
if (field.isEmpty())
@@ -223,7 +223,7 @@ class RecordTypeInfoImpl : public Base,
223223
return emitInitializeWithCopyCall(IGF, T, dest, src);
224224
}
225225

226-
if (isOutlined || T.hasOpenedExistential()) {
226+
if (isOutlined || T.hasLocalArchetype()) {
227227
auto offsets = asImpl().getNonFixedOffsets(IGF, T);
228228
for (auto &field : getFields()) {
229229
if (field.isEmpty())
@@ -255,7 +255,7 @@ class RecordTypeInfoImpl : public Base,
255255
return emitInitializeWithTakeCall(IGF, T, dest, src);
256256
}
257257

258-
if (isOutlined || T.hasOpenedExistential()) {
258+
if (isOutlined || T.hasLocalArchetype()) {
259259
auto offsets = asImpl().getNonFixedOffsets(IGF, T);
260260
for (auto &field : getFields()) {
261261
if (field.isEmpty())
@@ -278,7 +278,7 @@ class RecordTypeInfoImpl : public Base,
278278
return emitDestroyCall(IGF, T, addr);
279279
}
280280

281-
if (isOutlined || T.hasOpenedExistential()) {
281+
if (isOutlined || T.hasLocalArchetype()) {
282282
auto offsets = asImpl().getNonFixedOffsets(IGF, T);
283283
for (auto &field : getFields()) {
284284
if (field.isTriviallyDestroyable())

‎lib/IRGen/GenReflection.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,7 @@ class CaptureDescriptorBuilder : public ReflectionMetadataBuilder {
11661166

11671167
/// Give up if we captured an opened existential type. Eventually we
11681168
/// should figure out how to represent this.
1169-
static bool hasOpenedExistential(CanSILFunctionType OrigCalleeType,
1169+
static bool hasLocalArchetype(CanSILFunctionType OrigCalleeType,
11701170
const HeapLayout &Layout) {
11711171
if (!OrigCalleeType->isPolymorphic() ||
11721172
OrigCalleeType->isPseudogeneric())
@@ -1180,15 +1180,15 @@ class CaptureDescriptorBuilder : public ReflectionMetadataBuilder {
11801180
if (!Bindings[i].isAnyMetadata())
11811181
continue;
11821182

1183-
if (Bindings[i].getTypeParameter()->hasOpenedExistential())
1183+
if (Bindings[i].getTypeParameter()->hasLocalArchetype())
11841184
return true;
11851185
}
11861186

11871187
auto ElementTypes =
11881188
Layout.getElementTypes().slice(Layout.getIndexAfterBindings());
11891189
for (auto ElementType : ElementTypes) {
11901190
auto SwiftType = ElementType.getASTType();
1191-
if (SwiftType->hasOpenedExistential())
1191+
if (SwiftType->hasLocalArchetype())
11921192
return true;
11931193
}
11941194

@@ -1450,7 +1450,7 @@ IRGenModule::getAddrOfCaptureDescriptor(SILFunction &Caller,
14501450
if (IRGen.Opts.ReflectionMetadata != ReflectionMetadataMode::Runtime)
14511451
return llvm::Constant::getNullValue(CaptureDescriptorPtrTy);
14521452

1453-
if (CaptureDescriptorBuilder::hasOpenedExistential(OrigCalleeType, Layout))
1453+
if (CaptureDescriptorBuilder::hasLocalArchetype(OrigCalleeType, Layout))
14541454
return llvm::Constant::getNullValue(CaptureDescriptorPtrTy);
14551455

14561456
CaptureDescriptorBuilder builder(*this,

‎lib/IRGen/IRGenDebugInfo.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2579,8 +2579,8 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
25792579
if (Opts.DebugInfoLevel <= IRGenDebugInfoLevel::LineTables)
25802580
return;
25812581

2582-
// We cannot yet represent opened existentials.
2583-
if (DbgTy.getType()->hasOpenedExistential())
2582+
// We cannot yet represent local archetypes.
2583+
if (DbgTy.getType()->hasLocalArchetype())
25842584
return;
25852585

25862586
auto *Scope = dyn_cast_or_null<llvm::DILocalScope>(getOrCreateScope(DS));

‎lib/IRGen/Outlining.cpp

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -113,23 +113,26 @@ void OutliningMetadataCollector::bindMetadataParameters(IRGenFunction &IGF,
113113
std::pair<CanType, CanGenericSignature>
114114
irgen::getTypeAndGenericSignatureForManglingOutlineFunction(SILType type) {
115115
auto loweredType = type.getASTType();
116-
if (loweredType->hasArchetype()) {
117-
GenericEnvironment *env = nullptr;
118-
loweredType.findIf([&env](Type t) -> bool {
119-
if (auto arch = t->getAs<ArchetypeType>()) {
120-
if (!isa<PrimaryArchetypeType>(arch) &&
121-
!isa<VariadicSequenceType>(arch))
122-
return false;
123-
env = arch->getGenericEnvironment();
124-
return true;
125-
}
126-
return false;
127-
});
128-
assert(env && "has archetype but no archetype?!");
129-
return {loweredType->mapTypeOutOfContext()->getCanonicalType(),
130-
env->getGenericSignature().getCanonicalSignature()};
131-
}
132-
return {loweredType, nullptr};
116+
if (!loweredType->hasArchetype()) return {loweredType, nullptr};
117+
118+
// Find a non-local, non-opaque archetype in the type and pull out
119+
// its generic environment.
120+
// TODO: we ought to be able to usefully minimize this
121+
122+
GenericEnvironment *env = nullptr;
123+
loweredType.findIf([&env](CanType t) -> bool {
124+
if (auto arch = dyn_cast<ArchetypeType>(t)) {
125+
if (!isa<PrimaryArchetypeType>(arch) &&
126+
!isa<PackArchetypeType>(arch))
127+
return false;
128+
env = arch->getGenericEnvironment();
129+
return true;
130+
}
131+
return false;
132+
});
133+
assert(env && "has archetype but no archetype?!");
134+
return {loweredType->mapTypeOutOfContext()->getCanonicalType(),
135+
env->getGenericSignature().getCanonicalSignature()};
133136
}
134137

135138
void TypeInfo::callOutlinedCopy(IRGenFunction &IGF, Address dest, Address src,

‎lib/SIL/IR/SILType.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,12 @@ bool SILType::aggregateHasUnreferenceableStorage() const {
453453
if (auto s = getStructOrBoundGenericStruct()) {
454454
return s->hasUnreferenceableStorage();
455455
}
456+
// Tuples with pack expansions don't *actually* have unreferenceable
457+
// storage, but the optimizer needs to be taught how to handle them,
458+
// and it won't do that correctly in the short term.
459+
if (auto t = getAs<TupleType>()) {
460+
return t.containsPackExpansionType();
461+
}
456462
return false;
457463
}
458464

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ static void copyOrInitPackExpansionInto(SILGenFunction &SGF,
106106
// element value, that's fine, we'll just destroy it as part of
107107
// leaving the iteration.
108108
eltInit->copyOrInitValueInto(SGF, loc, eltMV, isInit);
109+
eltInit->finishInitialization(SGF);
109110

110111
// Deactivate the tail cleanup before continuing the loop.
111112
if (tailCleanup.isValid())

‎test/IRGen/variadic_generic_outlining.sil

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %target-swift-frontend -emit-ir -primary-file %s -enable-experimental-feature VariadicGenerics | %IRGenFileCheck %s
2+
3+
// Because of -enable-experimental-feature VariadicGenerics
4+
// REQUIRES: asserts
5+
6+
import Builtin
7+
import Swift
8+
9+
struct Wrapper<Value> {
10+
let value: Value
11+
}
12+
13+
// This specifically needs to bypass outlining because it involves
14+
// an opened element type.
15+
//
16+
// CHECK-LABEL: define{{.*}}void @test_outlining
17+
// CHECK: [[T0:%.*]] = getelementptr inbounds %swift.type*, %swift.type** %T, [[INT]]
18+
// CHECK-NEXT: [[ELT_TYPE:%.*]] = load %swift.type*, %swift.type** [[T0]], align
19+
// Test that we do the copy through the VWT for the element type.
20+
// CHECK: [[T0:%.*]] = bitcast %swift.type* [[ELT_TYPE]] to i8***
21+
// CHECK-NEXT: [[T1:%.*]] = getelementptr inbounds i8**, i8*** [[T0]], i64 -1
22+
sil hidden @test_outlining : $@convention(thin) <each T> (@pack_guaranteed Pack{repeat Wrapper<each T>}) -> @pack_out Pack{repeat Wrapper<each T>} {
23+
bb0(%0 : $*Pack{repeat Wrapper<each T>}, %1 : $*Pack{repeat Wrapper<each T>}):
24+
%zero = integer_literal $Builtin.Word, 0
25+
%one = integer_literal $Builtin.Word, 1
26+
%len = pack_length $Pack{repeat each T}
27+
br bb1(%zero : $Builtin.Word)
28+
29+
bb1(%idx : $Builtin.Word):
30+
%done = builtin "cmp_eq_Word"(%idx : $Builtin.Word, %len : $Builtin.Word) : $Builtin.Int1 // user: %10
31+
cond_br %done, bb3, bb2
32+
33+
bb2:
34+
%pi = dynamic_pack_index %idx of $Pack{repeat Wrapper<each T>}
35+
%opening = open_pack_element %pi of <each T> at <Pack{repeat each T}>, shape $T, uuid "31FF306C-BF88-11ED-A03F-ACDE48001122"
36+
%in = pack_element_get %pi of %0 : $*Pack{repeat Wrapper<each T>} as $*Wrapper<@pack_element("31FF306C-BF88-11ED-A03F-ACDE48001122") T>
37+
%out = pack_element_get %pi of %1 : $*Pack{repeat Wrapper<each T>} as $*Wrapper<@pack_element("31FF306C-BF88-11ED-A03F-ACDE48001122") T>
38+
copy_addr %in to [init] %out : $*Wrapper<@pack_element("31FF306C-BF88-11ED-A03F-ACDE48001122") T>
39+
%next = builtin "add_Word"(%idx : $Builtin.Word, %one : $Builtin.Word) : $Builtin.Word
40+
br bb1(%next : $Builtin.Word)
41+
42+
bb3:
43+
%ret = tuple ()
44+
return %ret : $()
45+
}

0 commit comments

Comments
 (0)