Skip to content

Commit 68b8322

Browse files
committed
[MoveOnlyAddressChecker] Handle enum deinits.
Previously, the enum representation was fixed to represent the different cases payloads separately with the unchecked_take_enum_data_addr instruction consuming all fields but that whose address is obtained. In a few places, handling for enum deinits was left undone with an assertion that the enum not have one. Here, the deinit bit of the enum is shifted to the end. And the assertions are replaced with handling. Finally, the logic for inserting destroys after switch_enum_addr instructions is fixed.
1 parent ce59be7 commit 68b8322

File tree

2 files changed

+91
-39
lines changed

2 files changed

+91
-39
lines changed

include/swift/SIL/FieldSensitivePrunedLiveness.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,11 @@ struct SubElementOffset {
239239
/// s a struct => count(s) := sum(s.fields, { f in count(type(f)) })
240240
/// + s.hasDeinit
241241
/// e an enum => count(e) := sum(e.elements, { elt in count(type(elt)) })
242-
/// + e.hasDeinit
243242
/// + 1 // discriminator
243+
/// + e.hasDeinit
244+
///
245+
/// The deinit bit is at the end to make drop_deinit produce a value whose
246+
/// leaves are contiguous.
244247
struct TypeSubElementCount {
245248
unsigned number;
246249

lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

Lines changed: 87 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,39 @@ SubElementOffset::computeForValue(SILValue projectionDerivedFromRoot,
375375
// MARK: TypeTreeLeafTypeRange
376376
//===----------------------------------------------------------------------===//
377377

378+
/// Whether \p targetInst is dominated by one of the provided switch_enum_addr's
379+
/// destination blocks whose corresponding enum element has no associated values
380+
/// which need to be destroyed (i.e. either it has no associated values or they
381+
/// are trivial).
382+
static bool isDominatedByPayloadlessSwitchEnumAddrDests(
383+
SILInstruction *targetInst, ArrayRef<SwitchEnumAddrInst *> seais,
384+
DominanceInfo *domTree) {
385+
if (seais.empty())
386+
return false;
387+
auto *target = targetInst->getParent();
388+
for (auto *seai : seais) {
389+
if (!domTree->dominates(seai, targetInst)) {
390+
continue;
391+
}
392+
auto size = seai->getNumCases();
393+
auto ty = seai->getOperand()->getType();
394+
for (unsigned index = 0; index < size; ++index) {
395+
auto pair = seai->getCase(index);
396+
auto *eltDecl = pair.first;
397+
if (eltDecl->hasAssociatedValues()) {
398+
auto eltTy = ty.getEnumElementType(eltDecl, seai->getFunction());
399+
if (!eltTy.isTrivial(*seai->getFunction())) {
400+
continue;
401+
}
402+
}
403+
auto *block = pair.second;
404+
if (domTree->dominates(block, target))
405+
return true;
406+
}
407+
}
408+
return false;
409+
}
410+
378411
void TypeTreeLeafTypeRange::constructFilteredProjections(
379412
SILValue value, SILInstruction *insertPt, SmallBitVector &filterBitVector,
380413
DominanceInfo *domTree,
@@ -429,60 +462,72 @@ void TypeTreeLeafTypeRange::constructFilteredProjections(
429462
unsigned next;
430463
};
431464
SmallVector<ElementRecord, 2> projectedElements;
432-
unsigned start = startEltOffset;
465+
unsigned runningStart = startEltOffset;
433466
for (auto *eltDecl : enumDecl->getAllElements()) {
434467
if (!eltDecl->hasAssociatedValues())
435468
continue;
436469

437-
auto nextType = type.getEnumElementType(eltDecl, fn);
438-
unsigned next = start + TypeSubElementCount(nextType, fn);
439-
if (noneSet(filterBitVector, start, next)) {
440-
start = next;
470+
auto eltTy = type.getEnumElementType(eltDecl, fn);
471+
unsigned next = runningStart + TypeSubElementCount(eltTy, fn);
472+
if (noneSet(filterBitVector, runningStart, next)) {
473+
runningStart = next;
441474
continue;
442475
}
443476

444-
projectedElements.push_back({eltDecl, start, next});
445-
start = next;
477+
projectedElements.push_back({eltDecl, runningStart, next});
478+
runningStart = next;
446479
}
480+
assert((runningStart + 1 + (type.isValueTypeWithDeinit() ? 1 : 0)) ==
481+
endEltOffset);
447482

448-
// Add a bit for the discriminator.
449-
unsigned next = start + 1;
450-
451-
if (!allSet(filterBitVector, start, next)) {
483+
if (!allSet(filterBitVector, startEltOffset, endEltOffset)) {
484+
TinyPtrVector<SwitchEnumAddrInst *> seais;
452485
for (auto record : projectedElements) {
453486
// Find a preexisting unchecked_take_enum_data_addr that dominates
454487
// insertPt.
455488
bool foundProjection = false;
456-
for (auto *user : value->getUsers()) {
457-
auto *utedai = dyn_cast<UncheckedTakeEnumDataAddrInst>(user);
458-
if (!utedai) {
459-
continue;
460-
}
461-
if (utedai->getElement() == record.element) {
462-
continue;
489+
StackList<SILValue> worklist(value->getFunction());
490+
worklist.push_back(value);
491+
while (!worklist.empty()) {
492+
auto v = worklist.pop_back_val();
493+
for (auto *user : v->getUsers()) {
494+
if (auto *ddi = dyn_cast<DropDeinitInst>(user)) {
495+
worklist.push_back(ddi);
496+
continue;
497+
}
498+
if (auto *seai = dyn_cast<SwitchEnumAddrInst>(user)) {
499+
seais.push_back(seai);
500+
}
501+
auto *utedai = dyn_cast<UncheckedTakeEnumDataAddrInst>(user);
502+
if (!utedai) {
503+
continue;
504+
}
505+
if (utedai->getElement() != record.element) {
506+
continue;
507+
}
508+
if (!domTree->dominates(utedai, insertPt)) {
509+
continue;
510+
}
511+
512+
callback(utedai, TypeTreeLeafTypeRange(record.start, record.next),
513+
NeedsDestroy);
514+
foundProjection = true;
463515
}
464-
if (!domTree->dominates(utedai, insertPt)) {
465-
continue;
466-
}
467-
468-
callback(utedai, TypeTreeLeafTypeRange(record.start, record.next),
469-
DoesNotNeedDestroy);
470-
foundProjection = true;
471516
}
517+
(void)foundProjection;
472518
assert(foundProjection ||
473-
llvm::count_if(enumDecl->getAllElements(), [](auto *elt) {
474-
return elt->hasAssociatedValues();
475-
}) == 1);
519+
llvm::count_if(
520+
enumDecl->getAllElements(),
521+
[](auto *elt) { return elt->hasAssociatedValues(); }) == 1 ||
522+
isDominatedByPayloadlessSwitchEnumAddrDests(insertPt, seais,
523+
domTree));
476524
}
477525
return;
478526
}
479527

480528
// Then just pass back our enum base value as the pointer.
481-
callback(value, TypeTreeLeafTypeRange(start, next), NeedsDestroy);
482-
483-
// Then set start to next and assert we covered the entire end elt offset.
484-
start = next;
485-
assert(start == endEltOffset);
529+
callback(value, TypeTreeLeafTypeRange(startEltOffset, endEltOffset),
530+
NeedsDestroy);
486531
return;
487532
}
488533

@@ -526,9 +571,12 @@ void TypeTreeLeafTypeRange::get(
526571

527572
// An `inject_enum_addr` only initializes the enum tag.
528573
if (auto inject = dyn_cast<InjectEnumAddrInst>(op->getUser())) {
529-
auto upperBound = *startEltOffset + TypeSubElementCount(projectedValue);
530-
// TODO: account for deinit component if enum has deinit.
531-
assert(!projectedValue->getType().isValueTypeWithDeinit());
574+
// Subtract the deinit bit, if any: the discriminator bit is before it:
575+
//
576+
// [ case1 bits ..., case2 bits, ..., discriminator bit, deinit bit ]
577+
auto deinitBits = projectedValue->getType().isValueTypeWithDeinit() ? 1 : 0;
578+
auto upperBound =
579+
*startEltOffset + TypeSubElementCount(projectedValue) - deinitBits;
532580
ranges.push_back({upperBound - 1, upperBound});
533581
return;
534582
}
@@ -551,10 +599,11 @@ void TypeTreeLeafTypeRange::get(
551599
}
552600
numAtoms += elementAtoms;
553601
}
554-
// TODO: account for deinit component if enum has deinit.
555-
assert(!projectedValue->getType().isValueTypeWithDeinit());
602+
// The discriminator bit is consumed.
556603
ranges.push_back(
557604
{*startEltOffset + numAtoms, *startEltOffset + numAtoms + 1});
605+
// The deinit bit is _not_ consumed. A drop_deinit is required to
606+
// consumingly switch an enum with a deinit.
558607
return;
559608
}
560609

0 commit comments

Comments
 (0)