Skip to content

Commit ca663b1

Browse files
committed
[region-isolation] Make it possible to "lookThrough" multiple result instructions and use it to lookthrough destructures.
Currently when one says that an instruction is not a "look through" instruction, each of its results gets a separate element number and we track these results as independent entities that can be in a region. The one issue with this is whenever we perform this sort of operation we actually are at the same time performing a require on the operand of the instruction. This causes us to emit errors on non-side effect having instructions when we really want to emit an error on their side-effect having results. As an example of the world before this patch, the following example would force the struct_element_addr to have a require so we would emit an error on it instead of the apply (the thing that we actually care about): ``` %0 = ... // We transferred %0, so we cannot use it again. apply %transfer(%0) // We track %1 and %0 as separate elements and we treat this as an assignment of // %0 into %1 which forces %0 to be required live at this point causing us to // emit an error here... %1 = struct_element_addr %0 // Instead of in the SIL here on the actual side-effect having instruction. apply %actualUse(%1) ``` the solution is to make instructions like struct_element_addr lookthrough instructions which force their result to just be the same element as their operand. As part of doing this, we have to ensure that getUnderlyingTrackedValue knows how to look through these types. This ensures that they are not considered roots. ---- As an aside to implement this I needed to compose some functionality ontop of getUnderlyingObject (specifically the look through behavior on destructures) in a new helper routine called getUnderlyingTrackedObjectValue(). It just in a loop calls getUnderlyingObject() and looks through destructures until its iterator doesn't change.
1 parent 44e1e54 commit ca663b1

File tree

1 file changed

+44
-10
lines changed

1 file changed

+44
-10
lines changed

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,34 @@ struct UseDefChainVisitor
140140

141141
} // namespace
142142

143+
static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
144+
SILValue result = value;
145+
while (true) {
146+
SILValue temp = result;
147+
148+
temp = getUnderlyingObject(temp);
149+
150+
if (auto *dsi = dyn_cast_or_null<DestructureStructInst>(
151+
temp->getDefiningInstruction())) {
152+
temp = dsi->getOperand();
153+
}
154+
if (auto *dti = dyn_cast_or_null<DestructureTupleInst>(
155+
temp->getDefiningInstruction())) {
156+
temp = dti->getOperand();
157+
}
158+
159+
if (temp != result) {
160+
result = temp;
161+
continue;
162+
}
163+
164+
return result;
165+
}
166+
}
167+
143168
static SILValue getUnderlyingTrackedValue(SILValue value) {
144169
if (!value->getType().isAddress()) {
145-
return getUnderlyingObject(value);
170+
return getUnderlyingTrackedObjectValue(value);
146171
}
147172

148173
UseDefChainVisitor visitor;
@@ -837,6 +862,17 @@ class PartitionOpTranslator {
837862
builder.addAssignFresh(value->getRepresentative());
838863
}
839864

865+
template <typename DestValues>
866+
void translateSILLookThrough(DestValues destValues, SILValue src) {
867+
auto srcID = tryToTrackValue(src);
868+
869+
for (SILValue dest : destValues) {
870+
auto destID = tryToTrackValue(dest);
871+
assert(((!destID || !srcID) || destID->getID() == srcID->getID()) &&
872+
"srcID and dstID are different?!");
873+
}
874+
}
875+
840876
/// Add a look through operation. This asserts that dest and src map to the
841877
/// same ID. Should only be used on instructions that are always guaranteed to
842878
/// have this property due to getUnderlyingTrackedValue looking through them.
@@ -846,7 +882,8 @@ class PartitionOpTranslator {
846882
/// explicit. Previously, we always called translateSILAssign and relied on
847883
/// the builder to recognize these cases and not create an assign
848884
/// PartitionOp. Doing such a thing obscures what is actually happening.
849-
void translateSILLookThrough(SILValue dest, SILValue src) {
885+
template <>
886+
void translateSILLookThrough<SILValue>(SILValue dest, SILValue src) {
850887
auto srcID = tryToTrackValue(src);
851888
auto destID = tryToTrackValue(dest);
852889
assert(((!destID || !srcID) || destID->getID() == srcID->getID()) &&
@@ -1059,6 +1096,11 @@ class PartitionOpTranslator {
10591096
case SILInstructionKind::UpcastInst:
10601097
return translateSILLookThrough(inst->getResult(0), inst->getOperand(0));
10611098

1099+
// We identify tuple results with their operand's id.
1100+
case SILInstructionKind::DestructureTupleInst:
1101+
case SILInstructionKind::DestructureStructInst:
1102+
return translateSILLookThrough(inst->getResults(), inst->getOperand(0));
1103+
10621104
case SILInstructionKind::UnconditionalCheckedCastInst:
10631105
if (SILDynamicCastInst(inst).isRCIdentityPreserving())
10641106
return translateSILLookThrough(inst->getResult(0), inst->getOperand(0));
@@ -1133,14 +1175,6 @@ class PartitionOpTranslator {
11331175
case SILInstructionKind::TryApplyInst:
11341176
return translateSILApply(inst);
11351177

1136-
// These are used by SIL to disaggregate values together in a gep like
1137-
// way. We want to error on uses, not on the destructure itself, so we
1138-
// propagate.
1139-
case SILInstructionKind::DestructureTupleInst:
1140-
case SILInstructionKind::DestructureStructInst:
1141-
return translateSILMultiAssign(inst->getResults(),
1142-
inst->getOperandValues());
1143-
11441178
// These are used by SIL to aggregate values together in a gep like way. We
11451179
// want to look at uses of structs, not the struct uses itself. So just
11461180
// propagate.

0 commit comments

Comments
 (0)