Skip to content

Commit ce9013a

Browse files
rjmccallrjmccall
committed
Fix IRGen to not use outlining for types with any kind of local archetype.
Previously it was testing for opened existentials specifically. We should really teach outlining to handle local archetypes properly. We'd have to build a generic signature for the lowered type, and that probably means also adding requirements that are relevant to value operations, but it would mean outlining would benefit all types, and it would let us avoid bundling in unnecessary information from the enclosing generic environment. A minor side-effect of this is that we no longer bind names to opened element type values. The names were things like \tau_1_0, which is not very useful, especially in LLVM IR where \tau is printed with two UTF-8 escapes.
1 parent 0e93232 commit ce9013a

File tree

9 files changed

+105
-50
lines changed

9 files changed

+105
-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,

‎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+
}

‎test/IRGen/variadic_generics.sil

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ entry(%addr : $*T):
148148

149149
// CHECK-LABEL: define {{.*}}@test_tuple_pack_element_addr_1(
150150
// CHECK-SAME: {{.*}}* nocapture [[TUPLE_ADDR:%[^,]+]], i{{(64|32)}} [[INDEX:%[^,]+]]
151+
// CHECK: [[ELT_TYPE:%.*]] = phi %swift.type* [
151152
// CHECK: [[RESPONSE:%[^,]+]] = call swiftcc %swift.metadata_response @swift_getTupleTypeMetadata
152153
// CHECK: [[UNCAST_METADATA:%[^,]+]] = extractvalue %swift.metadata_response [[RESPONSE]], 0
153154
// CHECK: [[METADATA:%[^,]+]] = bitcast %swift.type* [[UNCAST_METADATA]] to %swift.tuple_type*
@@ -156,7 +157,7 @@ entry(%addr : $*T):
156157
// CHECK: [[CAST_TUPLE_ADDR:%[^,]+]] = bitcast <{ %TSS }>* [[TUPLE_ADDR]] to i8*
157158
// CHECK: [[UNCAST_ELEMENT_ADDR:%[^,]+]] = getelementptr inbounds i8, i8* [[CAST_TUPLE_ADDR]], i32 [[OFFSET]]
158159
// CHECK: [[ELEMENT_ADDR:%[^,]+]] = bitcast i8* [[UNCAST_ELEMENT_ADDR]] to %swift.opaque*
159-
// CHECK: call swiftcc void @borrow(%swift.opaque* noalias nocapture [[ELEMENT_ADDR]], %swift.type* %"\CF\84_1_0")
160+
// CHECK: call swiftcc void @borrow(%swift.opaque* noalias nocapture [[ELEMENT_ADDR]], %swift.type* [[ELT_TYPE]])
160161
sil @test_tuple_pack_element_addr_1 : $<T, U> (@inout (String, T, U, Int), Builtin.Word) -> () {
161162
bb0(%tuple : $*(String, T, U, Int), %i : $Builtin.Word):
162163
%index = dynamic_pack_index %i of $Pack{Float, T, U, Float}

0 commit comments

Comments
 (0)