Skip to content

Commit 1102752

Browse files
nate-chandlernate-chandler
committed
[IRGen] Pack: Scope recursive pack allocations.
While materializing one metadata pack, another pack may need to be materialized. When that happens, the inner pack's dynamically sized allocation must be deallocated within the same dominance scope. The CFG within which the inner dynamically sized pack is allocated isn't visible from SIL; that explains why the existing infrastructure around `de`/`alloc_pack_metadata` instructions fails to produce a deallocation at the appropriate point. In the fullness of time, this emitted code should be optimized such that the inner loop is hoisted out of its current outer loop. rdar://141718098
1 parent 65bae74 commit 1102752

File tree

3 files changed

+94
-44
lines changed

3 files changed

+94
-44
lines changed

‎lib/IRGen/GenPack.cpp

Lines changed: 61 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -413,18 +413,17 @@ static void emitPackExpansionPack(
413413
IGF.Builder.emitBlock(loop);
414414
ConditionalDominanceScope condition(IGF);
415415

416-
auto *element = elementForIndex(phi);
416+
IGF.withLocalStackPackAllocs([&]() {
417+
auto *element = elementForIndex(phi);
417418

418-
// Store the element metadata into to the current destination index.
419-
auto *eltIndex = IGF.Builder.CreateAdd(dynamicIndex, phi);
420-
Address eltPtr(
421-
IGF.Builder.CreateInBoundsGEP(pack.getElementType(),
422-
pack.getAddress(),
423-
eltIndex),
424-
pack.getElementType(),
425-
pack.getAlignment());
419+
// Store the element metadata into to the current destination index.
420+
auto *eltIndex = IGF.Builder.CreateAdd(dynamicIndex, phi);
421+
Address eltPtr(IGF.Builder.CreateInBoundsGEP(pack.getElementType(),
422+
pack.getAddress(), eltIndex),
423+
pack.getElementType(), pack.getAlignment());
426424

427-
IGF.Builder.CreateStore(element, eltPtr);
425+
IGF.Builder.CreateStore(element, eltPtr);
426+
});
428427

429428
// Increment our counter.
430429
auto *next = IGF.Builder.CreateAdd(phi,
@@ -717,6 +716,20 @@ void IRGenFunction::eraseStackPackWitnessTableAlloc(StackAddress addr,
717716
(void)removed;
718717
}
719718

719+
void IRGenFunction::withLocalStackPackAllocs(llvm::function_ref<void()> fn) {
720+
auto oldSize = OutstandingStackPackAllocs.size();
721+
fn();
722+
SmallVector<StackPackAlloc, 2> allocs;
723+
for (auto index = oldSize, size = OutstandingStackPackAllocs.size();
724+
index < size; ++index) {
725+
allocs.push_back(OutstandingStackPackAllocs[index]);
726+
}
727+
while (OutstandingStackPackAllocs.size() > oldSize) {
728+
OutstandingStackPackAllocs.pop_back();
729+
}
730+
cleanupStackAllocPacks(*this, allocs);
731+
}
732+
720733
llvm::Value *irgen::emitWitnessTablePackRef(IRGenFunction &IGF,
721734
CanPackType packType,
722735
PackConformance *conformance) {
@@ -970,42 +983,46 @@ llvm::Value *irgen::emitTypeMetadataPackElementRef(
970983
auto *materialize = IGF.createBasicBlock("pack-index-element-metadata");
971984
IGF.Builder.emitBlock(materialize);
972985

973-
llvm::Value *metadata = nullptr;
974-
llvm::SmallVector<llvm::Value *, 2> wtables;
975-
wtables.reserve(conformances.size());
976-
if (auto expansionTy = dyn_cast<PackExpansionType>(elementTy)) {
977-
// Actually materialize %inner. Then use it to get the metadata from the
978-
// pack expansion at that index.
979-
auto *relativeIndex = IGF.Builder.CreateSub(index, lowerBound);
980-
auto context =
981-
OpenedElementContext::createForContextualExpansion(IGF.IGM.Context, expansionTy);
982-
auto patternTy = expansionTy.getPatternType();
983-
metadata = emitPackExpansionElementMetadata(IGF, context, patternTy,
984-
relativeIndex, request);
985-
for (auto conformance : conformances) {
986-
auto patternConformance = conformance.getPack()->getPatternConformances()[i];
987-
auto *wtable = emitPackExpansionElementWitnessTable(
988-
IGF, context, patternTy, patternConformance,
989-
&metadata, relativeIndex);
990-
wtables.push_back(wtable);
986+
IGF.withLocalStackPackAllocs([&]() {
987+
llvm::Value *metadata = nullptr;
988+
llvm::SmallVector<llvm::Value *, 2> wtables;
989+
wtables.reserve(conformances.size());
990+
if (auto expansionTy = dyn_cast<PackExpansionType>(elementTy)) {
991+
// Actually materialize %inner. Then use it to get the metadata from
992+
// the pack expansion at that index.
993+
auto *relativeIndex = IGF.Builder.CreateSub(index, lowerBound);
994+
auto context = OpenedElementContext::createForContextualExpansion(
995+
IGF.IGM.Context, expansionTy);
996+
auto patternTy = expansionTy.getPatternType();
997+
metadata = emitPackExpansionElementMetadata(IGF, context, patternTy,
998+
relativeIndex, request);
999+
for (auto conformance : conformances) {
1000+
auto patternConformance =
1001+
conformance.getPack()->getPatternConformances()[i];
1002+
auto *wtable = emitPackExpansionElementWitnessTable(
1003+
IGF, context, patternTy, patternConformance, &metadata,
1004+
relativeIndex);
1005+
wtables.push_back(wtable);
1006+
}
1007+
} else {
1008+
metadata = IGF.emitTypeMetadataRef(elementTy, request).getMetadata();
1009+
for (auto conformance : conformances) {
1010+
auto patternConformance =
1011+
conformance.getPack()->getPatternConformances()[i];
1012+
llvm::Value *_metadata = nullptr;
1013+
auto *wtable = emitWitnessTableRef(IGF, elementTy,
1014+
/*srcMetadataCache=*/&_metadata,
1015+
patternConformance);
1016+
wtables.push_back(wtable);
1017+
}
9911018
}
992-
} else {
993-
metadata = IGF.emitTypeMetadataRef(elementTy, request).getMetadata();
994-
for (auto conformance : conformances) {
995-
auto patternConformance = conformance.getPack()->getPatternConformances()[i];
996-
llvm::Value *_metadata = nullptr;
997-
auto *wtable =
998-
emitWitnessTableRef(IGF, elementTy, /*srcMetadataCache=*/&_metadata,
999-
patternConformance);
1000-
wtables.push_back(wtable);
1019+
metadataPhi->addIncoming(metadata, IGF.Builder.GetInsertBlock());
1020+
for (auto i : indices(wtables)) {
1021+
auto *wtable = wtables[i];
1022+
auto *wtablePhi = wtablePhis[i];
1023+
wtablePhi->addIncoming(wtable, IGF.Builder.GetInsertBlock());
10011024
}
1002-
}
1003-
metadataPhi->addIncoming(metadata, IGF.Builder.GetInsertBlock());
1004-
for (auto i : indices(wtables)) {
1005-
auto *wtable = wtables[i];
1006-
auto *wtablePhi = wtablePhis[i];
1007-
wtablePhi->addIncoming(wtable, IGF.Builder.GetInsertBlock());
1008-
}
1025+
});
10091026
IGF.Builder.CreateBr(exit);
10101027
// }} Finished emitting emit_i.
10111028

‎lib/IRGen/IRGenFunction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,8 @@ class IRGenFunction {
437437
void recordStackPackWitnessTableAlloc(StackAddress addr, llvm::Value *shape);
438438
void eraseStackPackWitnessTableAlloc(StackAddress addr, llvm::Value *shape);
439439

440+
void withLocalStackPackAllocs(llvm::function_ref<void()> fn);
441+
440442
/// Emit a load of a reference to the given Objective-C selector.
441443
llvm::Value *emitObjCSelectorRefLoad(StringRef selector);
442444

‎validation-test/IRGen/rdar141718098.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %target-run-simple-swift | %FileCheck %s
2+
// REQUIRES: executable_test
3+
4+
struct S<T> {
5+
let t: T
6+
func f<each A, each B>(
7+
_ b: S<(repeat each B)>
8+
) -> S<(repeat each A, repeat each B)>
9+
where T == (repeat each A) {
10+
S<(repeat each A, repeat each B)>(
11+
t: (repeat each t, repeat each b.t)
12+
)
13+
}
14+
}
15+
16+
func doit() {
17+
let s1 = S(t: ("", ""))
18+
// CHECK: S<(String, String)>(t: ("", ""))
19+
print(s1)
20+
let s2 = s1.f(S(t: (5, 5)))
21+
// CHECK: S<(String, String, Int, Int)>(t: ("", "", 5, 5))
22+
print(s2)
23+
let s3 = s2.f(S(t: (13.1, 26.2)))
24+
// CHECK: S<(String, String, Int, Int, Double, Double)>(t: ("", "", 5, 5, 13.1, 26.2))
25+
print(s3)
26+
let s4 = s3.f(S(t: (S(t: ""), S(t: ""))))
27+
// CHECK: S<(String, String, Int, Int, Double, Double, S<String>, S<String>)>(t: ("", "", 5, 5, 13.1, 26.2, main.S<Swift.String>(t: ""), main.S<Swift.String>(t: "")))
28+
print(s4)
29+
}
30+
31+
doit()

0 commit comments

Comments
 (0)