Skip to content

[ownership] Eliminate OperandOwnershipKindMap in favor of OwnershipConstraint #34683

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.

Sign up for GitHub

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 1 commit into from
Nov 11, 2020
Merged

[ownership] Eliminate OperandOwnershipKindMap in favor of OwnershipConstraint #34683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 36 additions & 1 deletion include/swift/SIL/OwnershipUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ class BorrowingOperandKind {
BeginBorrow,
BeginApply,
Branch,
Apply,
TryApply,
Yield,
};

private:
Expand All @@ -109,6 +112,12 @@ class BorrowingOperandKind {
return BorrowingOperandKind(BeginApply);
case SILInstructionKind::BranchInst:
return BorrowingOperandKind(Branch);
case SILInstructionKind::ApplyInst:
return BorrowingOperandKind(Apply);
case SILInstructionKind::TryApplyInst:
return BorrowingOperandKind(TryApply);
case SILInstructionKind::YieldInst:
return BorrowingOperandKind(Yield);
}
}

Expand Down Expand Up @@ -142,7 +151,8 @@ struct BorrowingOperand {
return *this;
}

/// If value is a borrow introducer return it after doing some checks.
/// If \p op is a borrow introducing operand return it after doing some
/// checks.
static Optional<BorrowingOperand> get(Operand *op) {
auto *user = op->getUser();
auto kind = BorrowingOperandKind::get(user->getKind());
Expand All @@ -151,6 +161,19 @@ struct BorrowingOperand {
return BorrowingOperand(*kind, op);
}

/// If \p op is a borrow introducing operand return it after doing some
/// checks.
static Optional<BorrowingOperand> get(const Operand *op) {
return get(const_cast<Operand *>(op));
}

/// If this borrowing operand results in the underlying value being borrowed
/// over a region of code instead of just for a single instruction, visit
/// those uses.
///
/// Example: An apply performs an instantaneous recursive borrow of a
/// guaranteed value but a begin_apply borrows the value over the entire
/// region of code corresponding to the coroutine.
void visitLocalEndScopeInstructions(function_ref<void(Operand *)> func) const;

/// Returns true if this borrow scope operand consumes guaranteed
Expand All @@ -159,6 +182,9 @@ struct BorrowingOperand {
switch (kind) {
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
case BorrowingOperandKind::Yield:
return false;
case BorrowingOperandKind::Branch:
return true;
Expand All @@ -170,8 +196,14 @@ struct BorrowingOperand {
/// for owned values.
bool canAcceptOwnedValues() const {
switch (kind) {
// begin_borrow can take any parameter
case BorrowingOperandKind::BeginBorrow:
// Yield can implicit borrow owned values.
case BorrowingOperandKind::Yield:
// FullApplySites can implicit borrow owned values.
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
return true;
case BorrowingOperandKind::Branch:
return false;
Expand All @@ -189,6 +221,9 @@ struct BorrowingOperand {
case BorrowingOperandKind::Branch:
return true;
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
case BorrowingOperandKind::Yield:
return false;
}
llvm_unreachable("Covered switch isn't covered?!");
Expand Down
188 changes: 47 additions & 141 deletions include/swift/SIL/SILValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,13 @@ struct OwnershipKind {
return OwnershipKind::None;
return *this;
}

/// Convert this ownership kind to a StringRef.
StringRef asString() const;
};

llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const OwnershipKind &kind);

/// A value representing the specific ownership semantics that a SILValue may
/// have.
struct ValueOwnershipKind {
Expand Down Expand Up @@ -566,150 +571,50 @@ inline bool ValueOwnershipKind::isCompatibleWith(SILValue other) const {
return isCompatibleWith(other.getOwnershipKind());
}

/// A map from a ValueOwnershipKind that an operand can accept to a
/// UseLifetimeConstraint that describes the effect that the operand's use has
/// on the underlying value. If a ValueOwnershipKind is not in this map then
/// matching an operand with the value results in an ill formed program.
///
/// So for instance, a map could specify that if a value is used as an owned
/// parameter, then the use implies that the original value is destroyed at that
/// point. In contrast, if the value is used as a guaranteed parameter, then the
/// liveness constraint just requires that the value remains alive at the use
/// point.
struct OperandOwnershipKindMap {
// One bit for if a value exists and if the value exists, what the
// ownership constraint is. These are stored as pairs.
//
// NOTE: We are burning 1 bit per unset value. But this is without
// matter since we are always going to need less bits than 64, so we
// should always have a small case SmallBitVector, so there is no
// difference in size.
static constexpr unsigned NUM_DATA_BITS =
2 * (unsigned(OwnershipKind::LastValueOwnershipKind) + 1);

/// A bit vector representing our "map". Given a ValueOwnershipKind k, if the
/// operand can accept k, the unsigned(k)*2 bit will be set to true. Assuming
/// that bit is set, the unsigned(k)*2+1 bit is set to the use lifetime
/// constraint provided by the value.
SmallBitVector data;

OperandOwnershipKindMap() : data(NUM_DATA_BITS) {}
OperandOwnershipKindMap(ValueOwnershipKind kind,
UseLifetimeConstraint constraint)
: data(NUM_DATA_BITS) {
add(kind, constraint);
}
class OwnershipConstraint {
OwnershipKind ownershipKind;
UseLifetimeConstraint lifetimeConstraint;

/// Return the OperandOwnershipKindMap that tests for compatibility with
/// ValueOwnershipKind kind. This means that it will accept a element whose
/// ownership is OwnershipKind::None.
static OperandOwnershipKindMap
compatibilityMap(ValueOwnershipKind kind, UseLifetimeConstraint constraint) {
OperandOwnershipKindMap set;
set.addCompatibilityConstraint(kind, constraint);
return set;
public:
OwnershipConstraint(OwnershipKind inputOwnershipKind,
UseLifetimeConstraint inputLifetimeConstraint)
: ownershipKind(inputOwnershipKind),
lifetimeConstraint(inputLifetimeConstraint) {
assert((ownershipKind != OwnershipKind::None ||
lifetimeConstraint == UseLifetimeConstraint::NonLifetimeEnding) &&
"ValueOwnershipKind::None can never have their lifetime ended");
}

/// Return a map that is compatible with any and all ValueOwnershipKinds
/// except for \p kind.
static OperandOwnershipKindMap
compatibleWithAllExcept(ValueOwnershipKind kind) {
OperandOwnershipKindMap map;
unsigned index = 0;
unsigned end = unsigned(OwnershipKind::LastValueOwnershipKind) + 1;
for (; index != end; ++index) {
if (ValueOwnershipKind(index) == kind) {
continue;
}
map.add(ValueOwnershipKind(index),
UseLifetimeConstraint::NonLifetimeEnding);
}
return map;
OwnershipKind getPreferredKind() const {
return ownershipKind;
}

/// Create a map that has compatibility constraints for each of the
/// ValueOwnershipKind, UseLifetimeConstraints in \p args.
static OperandOwnershipKindMap
compatibilityMap(std::initializer_list<
std::pair<ValueOwnershipKind, UseLifetimeConstraint>>
args) {
OperandOwnershipKindMap map;
for (auto &p : args) {
map.addCompatibilityConstraint(p.first, p.second);
}
return map;
bool isLifetimeEnding() const {
return lifetimeConstraint == UseLifetimeConstraint::LifetimeEnding;
}

/// Return a map that states that an operand can take any ownership with each
/// ownership having a must be live constraint.
static OperandOwnershipKindMap allLive() {
OperandOwnershipKindMap map;
unsigned index = 0;
unsigned end = unsigned(OwnershipKind::LastValueOwnershipKind) + 1;
while (index != end) {
map.add(ValueOwnershipKind(index),
UseLifetimeConstraint::NonLifetimeEnding);
++index;
}
return map;
UseLifetimeConstraint getLifetimeConstraint() const {
return lifetimeConstraint;
}

/// Specify that the operand associated with this set can accept a value with
/// ValueOwnershipKind \p kind. The value provided by the operand will have a
/// new ownership enforced constraint defined by \p constraint.
void add(ValueOwnershipKind kind, UseLifetimeConstraint constraint) {
unsigned index = unsigned(kind);
unsigned kindOffset = index * 2;
unsigned constraintOffset = index * 2 + 1;

// If we have already put this kind into the map, we require the constraint
// offset to be the same, i.e. we only allow for a kind to be added twice if
// the constraint is idempotent. We assert otherwise.
assert((!data[kindOffset] || UseLifetimeConstraint(bool(
data[constraintOffset])) == constraint) &&
"Adding kind twice to the map with different constraints?!");
data[kindOffset] = true;
data[constraintOffset] = bool(constraint);
static OwnershipConstraint anyValueAcceptingConstraint() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a lot of difficulty parsing names like this and attempting to sort out their meaning. It accepts any value? Obviously not. "Constraint" is already in the type name, so why is it repeated?

Whereas if it were simply any() or even unconstrained() I would immediately know what that means.

I would also know what "acceptsAnyOwnership" means, so that would be ok. It's still unnecessarily verbose because constraints always imply acceptance and Ownership is already in the name.

Starting from a minimal recognizable name, before adding any more words, the question should be "how could any reasonable person be misled by the shorter name, and how would adding more words fix that problem without creating more confusion"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that any works well.

return {OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding};
}

void addCompatibilityConstraint(ValueOwnershipKind kind,
UseLifetimeConstraint constraint) {
add(OwnershipKind::None, UseLifetimeConstraint::NonLifetimeEnding);
add(kind, constraint);
}
bool satisfiedBy(const Operand *use) const;

bool canAcceptKind(ValueOwnershipKind kind) const {
unsigned index = unsigned(kind);
unsigned kindOffset = index * 2;
return data[kindOffset];
bool satisfiesConstraint(ValueOwnershipKind testKind) const {
return ownershipKind.join(testKind) == testKind;
}

UseLifetimeConstraint getLifetimeConstraint(ValueOwnershipKind kind) const;

void print(llvm::raw_ostream &os) const;
SWIFT_DEBUG_DUMP;
bool operator==(const OwnershipConstraint &other) const {
return ownershipKind == other.ownershipKind &&
isLifetimeEnding() == other.isLifetimeEnding();
}
};

inline llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
OperandOwnershipKindMap map) {
map.print(os);
return os;
}

// Out of line to work around lack of forward declaration for operator <<.
inline UseLifetimeConstraint
OperandOwnershipKindMap::getLifetimeConstraint(ValueOwnershipKind kind) const {
#ifndef NDEBUG
if (!canAcceptKind(kind)) {
llvm::errs() << "Can not lookup lifetime constraint: " << kind
<< ". Not in map!\n"
<< *this;
llvm_unreachable("standard error assertion");
}
#endif
unsigned constraintOffset = unsigned(kind) * 2 + 1;
return UseLifetimeConstraint(data[constraintOffset]);
}
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
OwnershipConstraint constraint);

/// A formal SIL reference to a value, suitable for use as a stored
/// operand.
Expand Down Expand Up @@ -788,24 +693,25 @@ class Operand {
/// Return which operand this is in the operand list of the using instruction.
unsigned getOperandNumber() const;

/// Return the static map of ValueOwnershipKinds that this operand can
/// potentially have to the UseLifetimeConstraint associated with that
/// ownership kind
/// Return the ownership constraint that restricts what types of values this
/// Operand can contain. Returns none if the operand is a type dependent
/// operand.
///
/// NOTE: This is implemented in OperandOwnership.cpp.
OperandOwnershipKindMap getOwnershipKindMap() const;
Optional<OwnershipConstraint> getOwnershipConstraint() const;

/// Returns true if changing the operand to use a value with the given
/// ownership kind would not cause the operand to violate the operand's
/// ownership constraints. Returns false otherwise.
bool canAcceptKind(ValueOwnershipKind kind) const;

/// Returns true if this operand and its value satisfy the operand's
/// operand constraint.
bool satisfiesConstraints() const;

/// Returns true if this operand acts as a use that consumes its associated
/// value.
bool isLifetimeEnding() const {
// Type dependent uses can never be consuming and do not have valid
// ownership maps since they do not participate in the ownership system.
if (isTypeDependent())
return false;
auto map = getOwnershipKindMap();
auto constraint = map.getLifetimeConstraint(get().getOwnershipKind());
return constraint == UseLifetimeConstraint::LifetimeEnding;
}
bool isLifetimeEnding() const;

SILBasicBlock *getParentBlock() const;
SILFunction *getParentFunction() const;
Expand Down
Loading