Skip to content

Commit 3a1f58a

Browse files
committed
[region-isolation] Make sure that nonisolated(unsafe) works in all cases.
I made sure we match what we get without region isolation by turning off region isolation in one of the test runs on the test for this. There is one problem where for non-final classes with nonisolated(unsafe) var fields, we currently do not properly squelch since I need to do more infrastructure work. I am going to do that in the next commit. rdar://128299305
1 parent 89a2cfc commit 3a1f58a

File tree

8 files changed

+1295
-24
lines changed

8 files changed

+1295
-24
lines changed

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ class regionanalysisimpl::TrackableValueState {
163163
regionInfo = getIsolationRegionInfo().merge(newRegionInfo);
164164
}
165165

166+
void setDisconnectedNonisolatedUnsafe() {
167+
auto oldRegionInfo = getIsolationRegionInfo();
168+
assert(oldRegionInfo.isDisconnected());
169+
regionInfo = oldRegionInfo.withUnsafeNonIsolated();
170+
}
171+
166172
Element getID() const { return Element(id); }
167173

168174
void addFlag(TrackableValueFlag flag) { flagSet |= flag; }

include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,9 @@ SILValue createEmptyAndUndefValue(SILType ty, SILInstruction *insertionPoint,
616616
/// Check if a struct or its fields can have unreferenceable storage.
617617
bool findUnreferenceableStorage(StructDecl *decl, SILType structType,
618618
SILFunction *func);
619+
620+
SILValue getInitOfTemporaryAllocStack(AllocStackInst *asi);
621+
619622
} // end namespace swift
620623

621624
#endif // SWIFT_SILOPTIMIZER_UTILS_INSTOPTUTILS_H

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "swift/Basic/LLVM.h"
2020
#include "swift/SIL/SILFunction.h"
2121
#include "swift/SIL/SILInstruction.h"
22+
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
2223
#include "swift/SILOptimizer/Utils/SILIsolationInfo.h"
2324

2425
#include "llvm/ADT/MapVector.h"
@@ -963,6 +964,14 @@ struct PartitionOpEvaluator {
963964
return Impl::getIsolationInfo(partitionOp);
964965
}
965966

967+
std::optional<Element> getElement(SILValue value) const {
968+
return asImpl().getElement(value);
969+
}
970+
971+
SILValue getRepresentative(SILValue value) const {
972+
return asImpl().getRepresentative(value);
973+
}
974+
966975
/// Apply \p op to the partition op.
967976
void apply(const PartitionOp &op) const {
968977
if (shouldEmitVerboseLogging()) {
@@ -1018,10 +1027,16 @@ struct PartitionOpEvaluator {
10181027
assert(p.isTrackingElement(op.getOpArgs()[0]) &&
10191028
"Transfer PartitionOp's argument should already be tracked");
10201029

1030+
// Before we do any further work, see if we have a nonisolated(unsafe)
1031+
// element. In such a case, this is also not a real transfer point.
1032+
Element transferredElement = op.getOpArgs()[0];
1033+
if (getIsolationRegionInfo(transferredElement).isUnsafeNonIsolated()) {
1034+
return;
1035+
}
1036+
10211037
// Otherwise, we need to merge our isolation region info with the
10221038
// isolation region info of everything else in our region. This is the
10231039
// dynamic isolation region info found by the dataflow.
1024-
Element transferredElement = op.getOpArgs()[0];
10251040
Region transferredRegion = p.getRegion(transferredElement);
10261041
bool isClosureCapturedElt = false;
10271042
SILIsolationInfo transferredRegionIsolation;
@@ -1045,8 +1060,8 @@ struct PartitionOpEvaluator {
10451060
// region.
10461061
if (bool(transferredRegionIsolation) &&
10471062
!transferredRegionIsolation.isDisconnected()) {
1048-
return handleTransferNonTransferrable(op, op.getOpArgs()[0],
1049-
transferredRegionIsolation);
1063+
return handleTransferNonTransferrableHelper(op, op.getOpArgs()[0],
1064+
transferredRegionIsolation);
10501065
}
10511066

10521067
// Mark op.getOpArgs()[0] as transferred.
@@ -1136,6 +1151,24 @@ struct PartitionOpEvaluator {
11361151
return;
11371152
}
11381153

1154+
// If we have a temporary that is initialized with an unsafe nonisolated
1155+
// value... squelch the error like if we were that value.
1156+
//
1157+
// TODO: This goes away with opaque values.
1158+
if (SILValue equivalenceClassRep =
1159+
getRepresentative(transferringOp->get())) {
1160+
if (auto *asi = dyn_cast<AllocStackInst>(equivalenceClassRep)) {
1161+
if (SILValue value = getInitOfTemporaryAllocStack(asi)) {
1162+
if (auto elt = getElement(value)) {
1163+
SILIsolationInfo eltIsolationInfo = getIsolationRegionInfo(*elt);
1164+
if (eltIsolationInfo.isUnsafeNonIsolated()) {
1165+
return;
1166+
}
1167+
}
1168+
}
1169+
}
1170+
}
1171+
11391172
// If our instruction does not have any isolation info associated with it,
11401173
// it must be nonisolated. See if our function has a matching isolation to
11411174
// our transferring operand. If so, we can squelch this.
@@ -1151,6 +1184,35 @@ struct PartitionOpEvaluator {
11511184
// Ok, we actually need to emit a call to the callback.
11521185
return handleLocalUseAfterTransfer(op, elt, transferringOp);
11531186
}
1187+
1188+
// Private helper that squelches the error if our transfer instruction and our
1189+
// use have the same isolation.
1190+
void handleTransferNonTransferrableHelper(
1191+
const PartitionOp &op, Element elt,
1192+
SILIsolationInfo dynamicMergedIsolationInfo) const {
1193+
if (shouldTryToSquelchErrors()) {
1194+
// If we have a temporary that is initialized with an unsafe nonisolated
1195+
// value... squelch the error like if we were that value.
1196+
//
1197+
// TODO: This goes away with opaque values.
1198+
if (SILValue equivalenceClassRep =
1199+
getRepresentative(op.getSourceOp()->get())) {
1200+
if (auto *asi = dyn_cast<AllocStackInst>(equivalenceClassRep)) {
1201+
if (SILValue value = getInitOfTemporaryAllocStack(asi)) {
1202+
if (auto elt = getElement(value)) {
1203+
SILIsolationInfo eltIsolationInfo = getIsolationRegionInfo(*elt);
1204+
if (eltIsolationInfo.isUnsafeNonIsolated()) {
1205+
return;
1206+
}
1207+
}
1208+
}
1209+
}
1210+
}
1211+
}
1212+
1213+
// Ok, we actually need to emit a call to the callback.
1214+
return handleTransferNonTransferrable(op, elt, dynamicMergedIsolationInfo);
1215+
}
11541216
};
11551217

11561218
/// A base implementation that can be used to default initialize CRTP
@@ -1213,6 +1275,15 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
12131275
return SILIsolationInfo();
12141276
}
12151277

1278+
/// If we are able to, return the element associated with \p value. If
1279+
/// unsupported, returns none.
1280+
std::optional<Element> getElement(SILValue value) const { return {}; }
1281+
1282+
/// If supported, returns the representative in \p value's equivalence
1283+
/// class. Returns an empty SILValue if this is unsupported or if it does not
1284+
/// have one.
1285+
SILValue getRepresentative(SILValue value) const { return SILValue(); }
1286+
12161287
/// Check if the representative value of \p elt is closure captured at \p
12171288
/// op.
12181289
///

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "swift/SIL/SILInstruction.h"
3636
#include "swift/SIL/Test.h"
3737
#include "swift/SILOptimizer/PassManager/Transforms.h"
38+
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
3839
#include "swift/SILOptimizer/Utils/PartitionUtils.h"
3940
#include "swift/SILOptimizer/Utils/VariableNameUtils.h"
4041
#include "llvm/ADT/DenseMap.h"
@@ -384,6 +385,7 @@ static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
384385
assert(base);
385386
if (base->getType().isObject())
386387
return {getUnderlyingTrackedValue(base).value, visitor.actorIsolation};
388+
387389
return {base, visitor.actorIsolation};
388390
}
389391

@@ -2981,6 +2983,17 @@ bool BlockPartitionState::recomputeExitFromEntry(
29812983
return translator.getValueMap().getIsolationRegion(elt);
29822984
}
29832985

2986+
std::optional<Element> getElement(SILValue value) const {
2987+
return translator.getValueMap().getTrackableValue(value).getID();
2988+
}
2989+
2990+
SILValue getRepresentative(SILValue value) const {
2991+
return translator.getValueMap()
2992+
.getTrackableValue(value)
2993+
.getRepresentative()
2994+
.maybeGetValue();
2995+
}
2996+
29842997
bool isClosureCaptured(Element elt, Operand *op) const {
29852998
auto iter = translator.getValueForId(elt);
29862999
if (!iter)

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,6 +1616,17 @@ struct DiagnosticEvaluator final
16161616
return info->getValueMap().getIsolationRegion(element);
16171617
}
16181618

1619+
std::optional<Element> getElement(SILValue value) const {
1620+
return info->getValueMap().getTrackableValue(value).getID();
1621+
}
1622+
1623+
SILValue getRepresentative(SILValue value) const {
1624+
return info->getValueMap()
1625+
.getTrackableValue(value)
1626+
.getRepresentative()
1627+
.maybeGetValue();
1628+
}
1629+
16191630
bool isClosureCaptured(Element element, Operand *op) const {
16201631
auto value = info->getValueMap().maybeGetRepresentative(element);
16211632
if (!value)

0 commit comments

Comments
 (0)