Skip to content

[region-isolation] Do not look through begin_borrow or move_value if they are marked as a var_decl. #72959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
beaab91
[region-isolation] Add a small helper to TrackableValue to print it's…
gottesmm Apr 7, 2024
c9fe8ff
[region-isolation] Eliminate unnecessary using TrackableValueID = Ele…
gottesmm Apr 10, 2024
b80faee
[region-isolation] Rename Partition::{fresh_label,freshLabel}.
gottesmm Apr 10, 2024
20c2429
[region-isolation] Do not look through begin_borrow or move_value if …
gottesmm Apr 7, 2024
513ab78
[region-isolation] Move SILIsolationInfo determining code for ref_ele…
gottesmm Apr 9, 2024
2a7714a
[region-isolation] Move computation of SILIsolationInfo for FunctionR…
gottesmm Apr 10, 2024
baca235
[region-isolation] Change load [copy]/load_borrow to just use SILIsol…
gottesmm Apr 10, 2024
b407b21
[region-isolation] Finish moving isolation computation into SILIsolat…
gottesmm Apr 10, 2024
eed51e7
[region-isolation] Make load/load_borrow look through instructions.
gottesmm Apr 10, 2024
d8f39f7
[region-isolation] Begin tracking in SILIsolationInfo the actorInstan…
gottesmm Apr 10, 2024
51ef67d
[variable-name-utils] Refactor inferName/inferNameAndRoot helpers ont…
gottesmm Apr 10, 2024
62a4820
[region-isolation] When printing a SILIsolationInfo description for d…
gottesmm Apr 10, 2024
a9c163f
[region-isolation] Emit the correct error for closures that capture a…
gottesmm Apr 10, 2024
2e5b3bc
[region-isolation] Do not treat functions/class_methods that are isol…
gottesmm Apr 11, 2024
969dd69
[region-isolation] Fix thinko.
gottesmm Apr 11, 2024
9d965f8
Remove pragma clang optimize off that snuck in.
gottesmm Apr 12, 2024
fa20e52
[region-isolation] Add some tests around initializers and distributed…
gottesmm Apr 16, 2024
3c29997
[region-isolation] Out of an abundance of caution convert isActor -> …
gottesmm Apr 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions include/swift/SIL/SILType.h
Original file line number Diff line number Diff line change
Expand Up @@ -917,8 +917,16 @@ class SILType {

bool isMarkedAsImmortal() const;

/// Returns true if this type is an actor type. Returns false if this is any
/// other type. This includes distributed actors. To check for distributed
/// actors and actors, use isAnyActor().
bool isActor() const { return getASTType()->isActorType(); }

bool isDistributedActor() const { return getASTType()->isDistributedActor(); }

/// Returns true if this type is an actor or a distributed actor.
bool isAnyActor() const { return getASTType()->isAnyActorType(); }

/// Returns true if this function conforms to the Sendable protocol.
bool isSendable(SILFunction *fn) const;

Expand Down
20 changes: 11 additions & 9 deletions include/swift/SILOptimizer/Analysis/RegionAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class RegionAnalysisFunctionInfo;
namespace regionanalysisimpl {

using TransferringOperandSetFactory = Partition::TransferringOperandSetFactory;
using TrackableValueID = PartitionPrimitives::Element;
using Element = PartitionPrimitives::Element;
using Region = PartitionPrimitives::Region;

/// Check if the passed in type is NonSendable.
Expand Down Expand Up @@ -175,7 +175,7 @@ class regionanalysisimpl::TrackableValueState {

SILIsolationInfo getIsolationRegionInfo() const { return regionInfo; }

TrackableValueID getID() const { return TrackableValueID(id); }
Element getID() const { return Element(id); }

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

Expand All @@ -186,7 +186,7 @@ class regionanalysisimpl::TrackableValueState {
<< "][is_no_alias: " << (isNoAlias() ? "yes" : "no")
<< "][is_sendable: " << (isSendable() ? "yes" : "no")
<< "][region_value_kind: ";
getIsolationRegionInfo().print(os);
getIsolationRegionInfo().printForDiagnostics(os);
os << "].";
}

Expand Down Expand Up @@ -276,9 +276,7 @@ class regionanalysisimpl::TrackableValue {
return valueState.getIsolationRegionInfo();
}

TrackableValueID getID() const {
return TrackableValueID(valueState.getID());
}
Element getID() const { return Element(valueState.getID()); }

/// Return the representative value of this equivalence class of values.
RepresentativeValue getRepresentative() const { return representativeValue; }
Expand All @@ -289,6 +287,11 @@ class regionanalysisimpl::TrackableValue {
/// parameter.
bool isTransferringParameter() const;

void printIsolationInfo(SmallString<64> &outString) const {
llvm::raw_svector_ostream os(outString);
getIsolationRegionInfo().printForDiagnostics(os);
}

void print(llvm::raw_ostream &os) const {
os << "TrackableValue. State: ";
valueState.print(os);
Expand All @@ -315,7 +318,6 @@ class RegionAnalysisValueMap {
using Region = PartitionPrimitives::Region;
using TrackableValue = regionanalysisimpl::TrackableValue;
using TrackableValueState = regionanalysisimpl::TrackableValueState;
using TrackableValueID = Element;
using RepresentativeValue = regionanalysisimpl::RepresentativeValue;

private:
Expand Down Expand Up @@ -369,14 +371,14 @@ class RegionAnalysisValueMap {
SILInstruction *introducingInst) const;

private:
std::optional<TrackableValue> getValueForId(TrackableValueID id) const;
std::optional<TrackableValue> getValueForId(Element id) const;
std::optional<TrackableValue> tryToTrackValue(SILValue value) const;
TrackableValue
getActorIntroducingRepresentative(SILInstruction *introducingInst,
SILIsolationInfo isolation) const;
bool mergeIsolationRegionInfo(SILValue value, SILIsolationInfo isolation);
bool valueHasID(SILValue value, bool dumpIfHasNoID = false);
TrackableValueID lookupValueID(SILValue value);
Element lookupValueID(SILValue value);
};

class RegionAnalysisFunctionInfo {
Expand Down
71 changes: 53 additions & 18 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,24 @@ class SILIsolationInfo {

/// This is the value that we got isolation from if we were able to find
/// one. Used for isolation history.
SILValue isolationSource;
SILValue isolatedValue;

SILIsolationInfo(ActorIsolation actorIsolation, SILValue isolationSource)
/// If set this is the SILValue that represents the actor instance that we
/// derived isolatedValue from.
SILValue actorInstance;

SILIsolationInfo(ActorIsolation actorIsolation, SILValue isolatedValue,
SILValue actorInstance)
: kind(Actor), actorIsolation(actorIsolation),
isolationSource(isolationSource) {}
isolatedValue(isolatedValue), actorInstance(actorInstance) {
assert((!actorInstance ||
(actorIsolation.getKind() == ActorIsolation::ActorInstance &&
actorInstance->getType().isAnyActor())) &&
"actorInstance must be an actor if it is non-empty");
}

SILIsolationInfo(Kind kind, SILValue isolationSource)
: kind(kind), actorIsolation(), isolationSource(isolationSource) {}
SILIsolationInfo(Kind kind, SILValue isolatedValue)
: kind(kind), actorIsolation(), isolatedValue(isolatedValue) {}

SILIsolationInfo(Kind kind) : kind(kind), actorIsolation() {}

Expand All @@ -151,51 +161,68 @@ class SILIsolationInfo {

void printForDiagnostics(llvm::raw_ostream &os) const;

SWIFT_DEBUG_DUMPER(dumpForDiagnostics()) {
printForDiagnostics(llvm::dbgs());
llvm::dbgs() << '\n';
}

ActorIsolation getActorIsolation() const {
assert(kind == Actor);
return actorIsolation;
}

// If we are actor or task isolated and could find a specific value that
// caused the isolation, put it here. Used for isolation history.
/// If we are actor or task isolated and could find a specific value that
/// caused the isolation, put it here. Used for isolation history.
SILValue getIsolatedValue() const {
assert(kind == Task || kind == Actor);
return isolationSource;
return isolatedValue;
}

/// Return the specific SILValue for the actor that our isolated value is
/// isolated to if one exists.
SILValue getActorInstance() const {
assert(kind == Actor);
return actorInstance;
}

bool hasActorIsolation() const { return kind == Actor; }

bool hasIsolatedValue() const {
return (kind == Task || kind == Actor) && bool(isolationSource);
return (kind == Task || kind == Actor) && bool(isolatedValue);
}

[[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;

SILIsolationInfo withActorIsolated(SILValue isolatedValue,
SILValue actorInstance,
ActorIsolation isolation) {
return SILIsolationInfo::getActorIsolated(isolatedValue, isolation);
return SILIsolationInfo::getActorIsolated(isolatedValue, actorInstance,
isolation);
}

static SILIsolationInfo getDisconnected() { return {Kind::Disconnected}; }

static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
SILValue actorInstance,
ActorIsolation actorIsolation) {
return {actorIsolation, isolatedValue};
return {actorIsolation, isolatedValue, actorInstance};
}

static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
SILValue actorInstance,
NominalTypeDecl *typeDecl) {
if (typeDecl->isActor())
return {ActorIsolation::forActorInstanceSelf(typeDecl), isolatedValue};
if (typeDecl->isAnyActor())
return {ActorIsolation::forActorInstanceSelf(typeDecl), isolatedValue,
actorInstance};
auto isolation = swift::getActorIsolation(typeDecl);
if (isolation.isGlobalActor())
return {isolation, isolatedValue};
return {isolation, isolatedValue, actorInstance};
return {};
}

static SILIsolationInfo getGlobalActorIsolated(SILValue value,
Type globalActorType) {
return getActorIsolated(value,
return getActorIsolated(value, SILValue() /*no actor instance*/,
ActorIsolation::forGlobalActor(globalActorType));
}

Expand All @@ -207,7 +234,15 @@ class SILIsolationInfo {
static SILIsolationInfo get(SILInstruction *inst);

/// Attempt to infer the isolation region info for \p arg.
static SILIsolationInfo get(SILFunctionArgument *arg);
static SILIsolationInfo get(SILArgument *arg);

static SILIsolationInfo get(SILValue value) {
if (auto *arg = dyn_cast<SILArgument>(value))
return get(arg);
if (auto *inst = dyn_cast<SingleValueInstruction>(value))
return get(inst);
return {};
}

bool hasSameIsolation(ActorIsolation actorIsolation) const;

Expand Down Expand Up @@ -670,7 +705,7 @@ class Partition {

/// Track a label that is guaranteed to be strictly larger than all in use,
/// and therefore safe for use as a fresh label.
Region fresh_label = Region(0);
Region freshLabel = Region(0);

/// An immutable data structure that we use to push/pop isolation history.
IsolationHistory history;
Expand Down Expand Up @@ -841,7 +876,7 @@ class Partition {
llvm::dbgs() << "Partition";
if (canonical)
llvm::dbgs() << "(canonical)";
llvm::dbgs() << "(fresh=" << fresh_label << "){";
llvm::dbgs() << "(fresh=" << freshLabel << "){";
for (const auto &[i, label] : elementToRegionMap)
llvm::dbgs() << "[" << i << ": " << label << "] ";
llvm::dbgs() << "}\n";
Expand Down
9 changes: 9 additions & 0 deletions include/swift/SILOptimizer/Utils/VariableNameUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,15 @@ class VariableNameInferrer {

StringRef getName() const { return resultingString; }

/// Given a specific SILValue, construct a VariableNameInferrer and use it to
/// attempt to infer an identifier for the value.
static std::optional<Identifier> inferName(SILValue value);

/// Given a specific SILValue, construct a VariableNameInferrer and use it to
/// attempt to infer an identifier for the value and a named value.
static std::optional<std::pair<Identifier, SILValue>>
inferNameAndRoot(SILValue value);

private:
void drainVariableNamePath();
void popSingleVariableName();
Expand Down
Loading