Skip to content

Commit b1f6903

Browse files
gottesmmgottesmm
committed
[region-isolation] When assigning RValues into memory, use tuple_addr_constructor instead of doing it in pieces.
I also included changes to the rest of the SIL optimizer pipeline to ensure that the part of the optimizer pipeline before we lower tuple_addr_constructor (which is right after we run TransferNonSendable) work as before. The reason why I am doing this is that this ensures that diagnostic passes can tell the difference in between: ``` x = (a, b, c) ``` and ``` x.0 = a x.1 = b x.2 = c ``` This is important for things like TransferNonSendable where assigning over the entire tuple element is treated differently from if one were to initialize it in pieces using projections. rdar://117880194
1 parent d2b5bc3 commit b1f6903

19 files changed

+424
-77
lines changed

‎include/swift/SIL/SILCloner.h

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2184,11 +2184,29 @@ SILCloner<ImplClass>::visitTupleInst(TupleInst *Inst) {
21842184
template <typename ImplClass>
21852185
void SILCloner<ImplClass>::visitTupleAddrConstructorInst(
21862186
TupleAddrConstructorInst *Inst) {
2187-
auto Elements = getOpValueArray<8>(Inst->getElements());
2187+
SmallVector<SILValue, 8> Elements;
2188+
for (auto e : Inst->getElements()) {
2189+
SILValue mappedValue = getOpValue(e);
2190+
2191+
// Check if mappedValue only consists of empty tuple elements. If it does,
2192+
// then we do not add it to our result. This is because we know that the
2193+
// corresponding elements in getOpValue(Inst->getDest()) will also change
2194+
// into an empty exploded tuple. Since we only have leaf non-empty non-tuple
2195+
// elements as operands, these are not represented.
2196+
bool FoundNonTuple = false;
2197+
mappedValue->getType().getASTType().visit(
2198+
[&](CanType ty) { FoundNonTuple |= !ty->is<TupleType>(); });
2199+
if (FoundNonTuple)
2200+
Elements.push_back(mappedValue);
2201+
}
2202+
2203+
if (Elements.empty())
2204+
return;
2205+
21882206
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
21892207
recordClonedInstruction(Inst, getBuilder().createTupleAddrConstructor(
21902208
getOpLocation(Inst->getLoc()),
2191-
getOpValue(Inst->getDestValue()), Elements,
2209+
getOpValue(Inst->getDest()), Elements,
21922210
Inst->isInitializationOfDest()));
21932211
}
21942212

‎include/swift/SIL/SILInstruction.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6372,10 +6372,10 @@ class TupleAddrConstructorInst final
63726372
Dest = 0,
63736373
};
63746374

6375-
Operand &getDest() { return getAllOperands().front(); }
6376-
const Operand &getDest() const { return getAllOperands().front(); }
6375+
Operand &getDestOperand() { return getAllOperands().front(); }
6376+
const Operand &getDestOperand() const { return getAllOperands().front(); }
63776377

6378-
SILValue getDestValue() const { return getDest().get(); }
6378+
SILValue getDest() const { return getDestOperand().get(); }
63796379

63806380
/// The elements referenced by this TupleInst.
63816381
MutableArrayRef<Operand> getElementOperands() {
@@ -6392,14 +6392,16 @@ class TupleAddrConstructorInst final
63926392

63936393
unsigned getElementIndex(Operand *operand) {
63946394
assert(operand->getUser() == this);
6395-
assert(operand != &getDest() && "Cannot pass in the destination");
6395+
assert(operand != &getDestOperand() && "Cannot pass in the destination");
63966396
return operand->getOperandNumber() + 1;
63976397
}
63986398

63996399
unsigned getNumElements() const { return getTupleType()->getNumElements(); }
64006400

64016401
TupleType *getTupleType() const {
6402-
return getDest().get()->getType().getRawASTType()->castTo<TupleType>();
6402+
// We use getASTType() since we want to look through a wrapped noncopyable
6403+
// type to get to the underlying tuple type.
6404+
return getDest()->getType().getASTType()->castTo<TupleType>();
64036405
}
64046406

64056407
IsInitialization_t isInitializationOfDest() const {

‎lib/SIL/IR/SILPrinter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2223,7 +2223,7 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
22232223
} else {
22242224
*this << "[assign] ";
22252225
}
2226-
*this << getIDAndType(TI->getDestValue());
2226+
*this << getIDAndType(TI->getDest());
22272227

22282228
*this << " with (";
22292229

‎lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
750750
}
751751
case SILInstructionKind::TupleAddrConstructorInst: {
752752
auto *ca = cast<TupleAddrConstructorInst>(inst);
753-
impactType = ca->getDestValue()->getType();
753+
impactType = ca->getDest()->getType();
754754
if (!ca->isInitializationOfDest())
755755
return RuntimeEffect::MetaData | RuntimeEffect::Releasing;
756756
return RuntimeEffect::MetaData;

‎lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ void MemoryLifetimeVerifier::initDataflowInBlock(SILBasicBlock *block,
417417
if (elt->getType().isAddress())
418418
killBits(state, elt);
419419
}
420-
genBits(state, taci->getDestValue());
420+
genBits(state, taci->getDest());
421421
break;
422422
}
423423
case SILInstructionKind::DestroyAddrInst:

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3290,6 +3290,22 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
32903290
}
32913291
}
32923292

3293+
void checkTupleAddrConstructorInst(TupleAddrConstructorInst *taci) {
3294+
require(taci->getNumElements() > 0,
3295+
"Cannot be applied to tuples that do not contain any real "
3296+
"elements. E.x.: ((), ())");
3297+
for (auto elt : taci->getElements()) {
3298+
// We cannot have any elements that contain only tuple elements. This is
3299+
// due to our exploded representation. This means when specializing,
3300+
// cloners must eliminate these parameters.
3301+
bool hasNonTuple = false;
3302+
elt->getType().getASTType().visit([&](CanType ty) {
3303+
hasNonTuple |= !ty->is<TupleType>();
3304+
});
3305+
require(hasNonTuple, "Element only consists of tuples");
3306+
}
3307+
}
3308+
32933309
// Is a SIL type a potential lowering of a formal type?
32943310
bool isLoweringOf(SILType loweredType, CanType formalType) {
32953311
return loweredType.isLoweringOf(F.getTypeExpansionContext(), F.getModule(),

‎lib/SILGen/RValue.cpp

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -558,36 +558,33 @@ void RValue::copyInto(SILGenFunction &SGF, SILLocation loc,
558558
copyOrInitValuesInto<ImplodeKind::Copy>(I, elts, type, loc, SGF);
559559
}
560560

561-
static void assignRecursive(SILGenFunction &SGF, SILLocation loc,
562-
CanType type, ArrayRef<ManagedValue> &srcValues,
563-
SILValue destAddr) {
564-
// Recurse into tuples.
561+
void RValue::assignInto(SILGenFunction &SGF, SILLocation loc,
562+
SILValue destAddr) && {
563+
assert(isComplete() && "rvalue is not complete");
564+
assert(isPlusOneOrTrivial(SGF) && "Can not assign borrowed RValues");
565+
ArrayRef<ManagedValue> srcMvValues = values;
566+
567+
SWIFT_DEFER { assert(srcMvValues.empty() && "didn't claim all elements!"); };
568+
569+
// If we do not have a tuple, just bail early.
565570
auto srcTupleType = dyn_cast<TupleType>(type);
566-
if (srcTupleType && !srcTupleType.containsPackExpansionType()) {
567-
assert(destAddr->getType().castTo<TupleType>()->getNumElements()
568-
== srcTupleType->getNumElements());
569-
for (auto eltIndex : indices(srcTupleType.getElementTypes())) {
570-
auto eltDestAddr = SGF.B.createTupleElementAddr(loc, destAddr, eltIndex);
571-
assignRecursive(SGF, loc, srcTupleType.getElementType(eltIndex),
572-
srcValues, eltDestAddr);
573-
}
571+
if (!srcTupleType || srcTupleType.containsPackExpansionType()) {
572+
// Otherwise, pull the front value off the list.
573+
auto srcValue = srcMvValues.front();
574+
srcMvValues = srcMvValues.slice(1);
575+
srcValue.assignInto(SGF, loc, destAddr);
574576
return;
575577
}
576578

577-
// Otherwise, pull the front value off the list.
578-
auto srcValue = srcValues.front();
579-
srcValues = srcValues.slice(1);
579+
assert(destAddr->getType().castTo<TupleType>()->getNumElements() ==
580+
srcTupleType->getNumElements());
580581

581-
srcValue.assignInto(SGF, loc, destAddr);
582-
}
583-
584-
void RValue::assignInto(SILGenFunction &SGF, SILLocation loc,
585-
SILValue destAddr) && {
586-
assert(isComplete() && "rvalue is not complete");
587-
assert(isPlusOneOrTrivial(SGF) && "Can not assign borrowed RValues");
588-
ArrayRef<ManagedValue> srcValues = values;
589-
assignRecursive(SGF, loc, type, srcValues, destAddr);
590-
assert(srcValues.empty() && "didn't claim all elements!");
582+
// If we do have any srcMvValues, then emit a TupleAddrConstructor. If we do
583+
// not have any, then our tuple must consist only of empty tuples.
584+
if (srcMvValues.size())
585+
SGF.B.createTupleAddrConstructor(loc, destAddr, srcMvValues,
586+
IsNotInitialization);
587+
srcMvValues = ArrayRef<ManagedValue>();
591588
}
592589

593590
ManagedValue RValue::getAsSingleValue(SILGenFunction &SGF, SILLocation loc) && {

‎lib/SILGen/SILGenBuilder.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,19 @@ class SILGenBuilder : public SILBuilder {
505505
void createEndLifetime(SILLocation loc, ManagedValue selfValue) {
506506
createEndLifetime(loc, selfValue.forward(SGF));
507507
}
508+
509+
using SILBuilder::createTupleAddrConstructor;
510+
511+
void createTupleAddrConstructor(SILLocation loc, SILValue destAddr,
512+
ArrayRef<ManagedValue> elements,
513+
IsInitialization_t isInitOfDest) {
514+
SmallVector<SILValue, 8> values;
515+
for (auto mv : elements) {
516+
values.push_back(mv.forward(SGF));
517+
}
518+
519+
createTupleAddrConstructor(loc, destAddr, values, isInitOfDest);
520+
}
508521
};
509522

510523
} // namespace Lowering

‎lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,27 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
908908
continue;
909909
}
910910

911+
if (auto *TACI = dyn_cast<TupleAddrConstructorInst>(User)) {
912+
// If this is the source of the copy_addr, then this is a load. If it is
913+
// the destination, then this is an unknown assignment. Note that we'll
914+
// revisit this instruction and add it to Uses twice if it is both a load
915+
// and store to the same aggregate.
916+
DIUseKind Kind;
917+
if (TACI->getDest() == Op->get()) {
918+
if (InStructSubElement)
919+
Kind = DIUseKind::PartialStore;
920+
else if (TACI->isInitializationOfDest())
921+
Kind = DIUseKind::Initialization;
922+
else
923+
Kind = DIUseKind::InitOrAssign;
924+
} else {
925+
Kind = DIUseKind::Load;
926+
}
927+
928+
addElementUses(BaseEltNo, PointeeType, User, Kind);
929+
continue;
930+
}
931+
911932
if (auto *MAI = dyn_cast<MarkUnresolvedMoveAddrInst>(User)) {
912933
// If this is the source of the copy_addr, then this is a load. If it is
913934
// the destination, then this is an unknown assignment. Note that we'll

‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2529,6 +2529,30 @@ void LifetimeChecker::updateInstructionForInitState(unsigned UseID) {
25292529
return;
25302530
}
25312531

2532+
if (auto *TACI = dyn_cast<TupleAddrConstructorInst>(Inst)) {
2533+
assert(!TACI->isInitializationOfDest() &&
2534+
"should not modify copy_addr that already knows it is initialized");
2535+
TACI->setIsInitializationOfDest(InitKind);
2536+
if (InitKind == IsInitialization)
2537+
setStaticInitAccess(TACI->getDest());
2538+
2539+
// If we had an initialization and had an assignable_but_not_consumable
2540+
// noncopyable type, convert it to be an initable_but_not_consumable so that
2541+
// we do not consume an uninitialized value.
2542+
if (InitKind == IsInitialization) {
2543+
if (auto *mmci = dyn_cast<MarkUnresolvedNonCopyableValueInst>(
2544+
stripAccessMarkers(TACI->getDest()))) {
2545+
if (mmci->getCheckKind() == MarkUnresolvedNonCopyableValueInst::
2546+
CheckKind::AssignableButNotConsumable) {
2547+
mmci->setCheckKind(MarkUnresolvedNonCopyableValueInst::CheckKind::
2548+
InitableButNotConsumable);
2549+
}
2550+
}
2551+
}
2552+
2553+
return;
2554+
}
2555+
25322556
// Ignore non-stores for SelfInits.
25332557
assert(isa<StoreInst>(Inst) && "Unknown store instruction!");
25342558
}

‎lib/SILOptimizer/Mandatory/LowerTupleAddrConstructor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class LowerTupleAddrConstructorTransform : public SILFunctionTransform {
4040

4141
unsigned count = 0;
4242
visitExplodedTupleValue(
43-
inst->getDestValue(),
43+
inst->getDest(),
4444
[&](SILValue value, std::optional<unsigned> index) -> SILValue {
4545
if (!index) {
4646
SILValue elt = inst->getElement(count);

‎lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,14 @@ class PartitionOpTranslator {
860860
"srcID and dstID are different?!");
861861
}
862862

863-
void translateSILAssign(SILValue dest, SILValue src) {
864-
return translateSILMultiAssign(TinyPtrVector<SILValue>(dest),
865-
TinyPtrVector<SILValue>(src));
863+
template <typename Collection>
864+
void translateSILAssign(SILValue dest, Collection collection) {
865+
return translateSILMultiAssign(TinyPtrVector<SILValue>(dest), collection);
866+
}
867+
868+
template <>
869+
void translateSILAssign<SILValue>(SILValue dest, SILValue src) {
870+
return translateSILAssign(dest, TinyPtrVector<SILValue>(src));
866871
}
867872

868873
void translateSILAssign(SILInstruction *inst) {
@@ -877,13 +882,22 @@ class PartitionOpTranslator {
877882
TinyPtrVector<SILValue>());
878883
}
879884

880-
void translateSILMerge(SILValue dest, SILValue src) {
885+
template <typename Collection>
886+
void translateSILMerge(SILValue dest, Collection collection) {
881887
auto trackableDest = tryToTrackValue(dest);
882-
auto trackableSrc = tryToTrackValue(src);
883-
if (!trackableDest || !trackableSrc)
888+
if (!trackableDest)
884889
return;
885-
builder.addMerge(trackableDest->getRepresentative(),
886-
trackableSrc->getRepresentative());
890+
for (SILValue elt : collection) {
891+
if (auto trackableSrc = tryToTrackValue(elt)) {
892+
builder.addMerge(trackableDest->getRepresentative(),
893+
trackableSrc->getRepresentative());
894+
}
895+
}
896+
}
897+
898+
template <>
899+
void translateSILMerge<SILValue>(SILValue dest, SILValue src) {
900+
return translateSILMerge(dest, TinyPtrVector<SILValue>(src));
887901
}
888902

889903
/// If tgt is known to be unaliased (computed thropugh a combination of
@@ -914,6 +928,30 @@ class PartitionOpTranslator {
914928
// Stores to storage of non-Sendable type can be ignored.
915929
}
916930

931+
void translateSILTupleAddrConstructor(TupleAddrConstructorInst *inst) {
932+
SILValue dest = inst->getDest();
933+
if (auto nonSendableTgt = tryToTrackValue(dest)) {
934+
// In the following situations, we can perform an assign:
935+
//
936+
// 1. A store to unaliased storage.
937+
// 2. A store that is to an entire value.
938+
//
939+
// DISCUSSION: If we have case 2, we need to merge the regions since we
940+
// are not overwriting the entire region of the value. This does mean that
941+
// we artificially include the previous region that was stored
942+
// specifically in this projection... but that is better than
943+
// miscompiling. For memory like this, we probably need to track it on a
944+
// per field basis to allow for us to assign.
945+
if (nonSendableTgt.value().isNoAlias() && !isProjectedFromAggregate(dest))
946+
return translateSILAssign(dest, inst->getElements());
947+
948+
// Stores to possibly aliased storage must be treated as merges.
949+
return translateSILMerge(dest, inst->getElements());
950+
}
951+
952+
// Stores to storage of non-Sendable type can be ignored.
953+
}
954+
917955
void translateSILRequire(SILValue val) {
918956
if (auto nonSendableVal = tryToTrackValue(val))
919957
return builder.addRequire(nonSendableVal->getRepresentative());
@@ -1090,6 +1128,10 @@ class PartitionOpTranslator {
10901128
case SILInstructionKind::StoreWeakInst:
10911129
return translateSILStore(inst->getOperand(1), inst->getOperand(0));
10921130

1131+
case SILInstructionKind::TupleAddrConstructorInst:
1132+
return translateSILTupleAddrConstructor(
1133+
cast<TupleAddrConstructorInst>(inst));
1134+
10931135
// Applies are handled specially since we need to merge their results.
10941136
case SILInstructionKind::ApplyInst:
10951137
case SILInstructionKind::BeginApplyInst:

‎lib/Serialization/SerializeSIL.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2345,7 +2345,7 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) {
23452345
result |= value->getType().isObject() ? 0 : 0x80000000;
23462346
return result;
23472347
};
2348-
ListOfValues.push_back(getValue(TI->getDestValue()));
2348+
ListOfValues.push_back(getValue(TI->getDest()));
23492349
for (auto Elt : TI->getElements()) {
23502350
ListOfValues.push_back(getValue(Elt));
23512351
}
@@ -2354,7 +2354,7 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) {
23542354
options |= bool(TI->isInitializationOfDest());
23552355
SILOneTypeValuesCategoriesLayout::emitRecord(
23562356
Out, ScratchRecord, abbrCode, (unsigned)SI.getKind(),
2357-
S.addTypeRef(TI->getDestValue()->getType().getRawASTType()),
2357+
S.addTypeRef(TI->getDest()->getType().getRawASTType()),
23582358
(unsigned)SILValueCategory::Address, options, ListOfValues);
23592359
break;
23602360
}

0 commit comments

Comments
 (0)