averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-21 12:14:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

[region-isolation] Fix handling of coroutine apply results.

Browse Source 
In this part of the code, we are attempting to merge all of the operands into
the same region and then assigning all non-Sendable results of the function to
that same region. The problem that was occuring here was a thinko due to the
control flow of the code here not separating nicely the case of whether or not
we had operands or not. Previously this did not matter, since we just used the
first result in such a case... but since we changed to assign to the first
operand element in some cases, it matters now. To fix this, I split the confused
logic into two different easy to follow control paths... one if we have operands
and one where we do not have an operand. In the case where we have a first
operand, we merge our elements into its region. If we do not have any operands,
then we just perform one large region assign fresh.

This was not exposed by code that used non-coroutines since in SIL only
coroutines today have multiple results.

rdar://132767643
This commit is contained in:
Michael Gottesman
2024-07-30 11:06:16 -07:00
parent c1ddeb26c1
commit 541863dbc6
3 changed files with 62 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
include/swift/SILOptimizer/Utils/PartitionUtils.h
View File

@@ -493,6 +493,19 @@ private:
"Transfer needs a sourceInst");
}
template <typename T>
PartitionOp(PartitionOpKind opKind, T collectionOfIndices,
SILInstruction *sourceInst = nullptr)
: opKind(opKind), opArgs(), source(sourceInst) {
assert(((opKind != PartitionOpKind::Transfer &&
opKind != PartitionOpKind::UndoTransfer) ||
sourceInst) &&
"Transfer needs a sourceInst");
for (Element elt : collectionOfIndices) {
opArgs.push_back(elt);
}
}
PartitionOp(PartitionOpKind opKind, Element arg1, Operand *sourceOperand)
: opKind(opKind), opArgs({arg1}), source(sourceOperand) {
assert(((opKind != PartitionOpKind::Transfer &&
@@ -529,9 +542,10 @@ public:
return PartitionOp(PartitionOpKind::Assign, destElt, srcElt, srcOperand);
}
static PartitionOp AssignFresh(Element tgt,
template <typename T>
static PartitionOp AssignFresh(T collection,
SILInstruction *sourceInst = nullptr) {
return PartitionOp(PartitionOpKind::AssignFresh, tgt, sourceInst);
return PartitionOp(PartitionOpKind::AssignFresh, collection, sourceInst);
}
static PartitionOp Transfer(Element tgt, Operand *transferringOp) {
@@ -1162,12 +1176,18 @@ public:
p.assignElement(op.getOpArgs()[0], op.getOpArgs()[1]);
return;
}
case PartitionOpKind::AssignFresh:
assert(op.getOpArgs().size() == 1 &&
"AssignFresh PartitionOp should be passed 1 argument");
case PartitionOpKind::AssignFresh: {
auto arrayRef = op.getOpArgs();
p.trackNewElement(op.getOpArgs()[0]);
Element front = arrayRef.front();
p.trackNewElement(front);
arrayRef = arrayRef.drop_front();
for (auto x : arrayRef) {
p.trackNewElement(x);
p.assignElement(x, front);
}
return;
}
case PartitionOpKind::Transfer: {
// NOTE: We purposely do not check here if a transferred value is already
// transferred. Callers are expected to put a require for that