Skip to content

Commit 48cc5b8

Browse files
committed
[region-isolation] Refactor RegionAnalysis AST pattern matching onto a helper factory method on IsolationRegionInfo.
Long term I would like to get region analysis and transfer non sendable out of the business of directly interpreting the AST... but if we have to do it now, I would rather us do it through a helper struct. At least the helper struct can be modified later to work with additional SIL concurrency support when it is added.
1 parent 70f94e3 commit 48cc5b8

File tree

3 files changed

+42
-28
lines changed

3 files changed

+42
-28
lines changed

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,9 @@ class IsolationRegionInfo {
257257
return {Kind::Task, value};
258258
}
259259

260+
/// Attempt to infer the isolation region info for \p inst.
261+
static IsolationRegionInfo get(SILInstruction *inst);
262+
260263
bool operator==(const IsolationRegionInfo &other) const {
261264
if (getKind() != other.getKind())
262265
return false;

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,6 @@ using namespace swift::regionanalysisimpl;
4848
// MARK: Utilities
4949
//===----------------------------------------------------------------------===//
5050

51-
/// SILApplyCrossesIsolation determines if a SIL instruction is an isolation
52-
/// crossing apply expression. This is done by checking its correspondence to an
53-
/// ApplyExpr AST node, and then checking the internal flags of that AST node to
54-
/// see if the ActorIsolationChecker determined it crossed isolation. It's
55-
/// possible this is brittle and a more nuanced check is needed, but this
56-
/// suffices for all cases tested so far.
57-
static bool isIsolationBoundaryCrossingApply(SILInstruction *inst) {
58-
if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>())
59-
return apply->getIsolationCrossing().has_value();
60-
if (auto fas = FullApplySite::isa(inst)) {
61-
if (bool(fas.getIsolationCrossing()))
62-
return true;
63-
}
64-
65-
// We assume that any instruction that does not correspond to an ApplyExpr
66-
// cannot cross an isolation domain.
67-
return false;
68-
}
69-
7051
namespace {
7152

7253
struct UnderlyingTrackedValueInfo {
@@ -1776,8 +1757,6 @@ class PartitionOpTranslator {
17761757
}
17771758

17781759
void translateSILPartialApply(PartialApplyInst *pai) {
1779-
assert(!isIsolationBoundaryCrossingApply(pai));
1780-
17811760
// First check if our partial apply is Sendable. In such a case, we will
17821761
// have emitted an earlier warning in Sema.
17831762
//
@@ -1811,12 +1790,8 @@ class PartitionOpTranslator {
18111790
}
18121791
}
18131792

1814-
if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
1815-
auto actorIsolation = ace->getActorIsolation();
1816-
if (actorIsolation.isActorIsolated()) {
1817-
return translateIsolatedPartialApply(
1818-
pai, IsolationRegionInfo::getActorIsolated(actorIsolation));
1819-
}
1793+
if (auto isolationRegionInfo = IsolationRegionInfo::get(pai)) {
1794+
return translateIsolatedPartialApply(pai, isolationRegionInfo);
18201795
}
18211796

18221797
SmallVector<SILValue, 8> applyResults;
@@ -1928,9 +1903,11 @@ class PartitionOpTranslator {
19281903
}
19291904
}
19301905

1906+
auto isolationRegionInfo = IsolationRegionInfo::get(inst);
1907+
19311908
// If this apply does not cross isolation domains, it has normal
19321909
// non-transferring multi-assignment semantics
1933-
if (!isIsolationBoundaryCrossingApply(inst))
1910+
if (!bool(isolationRegionInfo))
19341911
return translateNonIsolationCrossingSILApply(fas);
19351912

19361913
if (auto cast = dyn_cast<ApplyInst>(inst))

lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,12 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "swift/SILOptimizer/Utils/PartitionUtils.h"
14+
#include "swift/AST/Expr.h"
15+
#include "swift/SIL/ApplySite.h"
1416
#include "llvm/Support/CommandLine.h"
1517

18+
using namespace swift;
19+
1620
//===----------------------------------------------------------------------===//
1721
// MARK: Logging
1822
//===----------------------------------------------------------------------===//
@@ -31,3 +35,33 @@ static llvm::cl::opt<bool, true> // The parser
3135
REGIONBASEDISOLATION_ENABLE_VERBOSE_LOGGING));
3236

3337
#endif
38+
39+
//===----------------------------------------------------------------------===//
40+
// MARK: IsolationRegionInfo
41+
//===----------------------------------------------------------------------===//
42+
43+
IsolationRegionInfo IsolationRegionInfo::get(SILInstruction *inst) {
44+
if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>())
45+
if (auto crossing = apply->getIsolationCrossing())
46+
return IsolationRegionInfo::getActorIsolated(
47+
crossing->getCalleeIsolation());
48+
49+
if (auto fas = FullApplySite::isa(inst)) {
50+
if (auto crossing = fas.getIsolationCrossing())
51+
return IsolationRegionInfo::getActorIsolated(
52+
crossing->getCalleeIsolation());
53+
}
54+
55+
if (auto *pai = dyn_cast<PartialApplyInst>(inst)) {
56+
if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
57+
auto actorIsolation = ace->getActorIsolation();
58+
if (actorIsolation.isActorIsolated()) {
59+
return IsolationRegionInfo::getActorIsolated(actorIsolation);
60+
}
61+
}
62+
}
63+
64+
// We assume that any instruction that does not correspond to an ApplyExpr
65+
// cannot cross an isolation domain.
66+
return IsolationRegionInfo();
67+
}

0 commit comments

Comments
 (0)