Skip to content

[ownership] Add a new ReborrowVerifier #34438

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 3, 2020
Merged

[ownership] Add a new ReborrowVerifier #34438

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
1 change: 1 addition & 0 deletions include/swift/SIL/LinearLifetimeChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class LinearLifetimeChecker {
class ErrorBuilder;

private:
friend class ReborrowVerifier;
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than doing this, can you expose a higher level API on LinearLifetimeChecker? Maybe an asserting variant of checkUses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Am doing this to pass LinearLifetimeChecker::ErrorBuilder with continuous error number with a common errorMessageCounter. I would have used the public LinearLifetimeChecker ::validateLifetime but would loose the nice error numbering.
What do you think about exposing a version of LinearLifetimeChecker ::validateLifetime that has an errorBuilder parameter ?

friend class SILOwnershipVerifier;
friend class SILValueOwnershipChecker;

Expand Down
7 changes: 5 additions & 2 deletions include/swift/SIL/OwnershipUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ bool isOwnedForwardingInstruction(SILInstruction *inst);
/// previous terminator.
bool isOwnedForwardingValue(SILValue value);

/// Returns true if the instruction is a 'reborrow'.
bool isReborrowInstruction(const SILInstruction *inst);

class BorrowingOperandKind {
public:
enum Kind : uint8_t {
Expand Down Expand Up @@ -134,7 +137,7 @@ struct BorrowingOperand {
return BorrowingOperand(*kind, op);
}

void visitEndScopeInstructions(function_ref<void(Operand *)> func) const;
void visitLocalEndScopeInstructions(function_ref<void(Operand *)> func) const;

/// Returns true if this borrow scope operand consumes guaranteed
/// values and produces a new scope afterwards.
Expand Down Expand Up @@ -204,7 +207,7 @@ struct BorrowingOperand {
/// \p errorFunction a callback that if non-null is passed an operand that
/// triggers a mal-formed SIL error. This is just needed for the ownership
/// verifier to emit good output.
bool getImplicitUses(
void getImplicitUses(
SmallVectorImpl<Operand *> &foundUses,
std::function<void(Operand *)> *errorFunction = nullptr) const;

Expand Down
101 changes: 99 additions & 2 deletions include/swift/SIL/SILValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class SILInstruction;
class SILLocation;
class DeadEndBlocks;
class ValueBaseUseIterator;
class ValueUseIterator;
class ConsumingUseIterator;
class NonConsumingUseIterator;
class SILValue;

/// An enumeration which contains values for all the concrete ValueBase
Expand Down Expand Up @@ -263,10 +264,20 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {

using use_iterator = ValueBaseUseIterator;
using use_range = iterator_range<use_iterator>;
using consuming_use_iterator = ConsumingUseIterator;
using consuming_use_range = iterator_range<consuming_use_iterator>;
using non_consuming_use_iterator = NonConsumingUseIterator;
using non_consuming_use_range = iterator_range<non_consuming_use_iterator>;

inline use_iterator use_begin() const;
inline use_iterator use_end() const;

inline consuming_use_iterator consuming_use_begin() const;
inline consuming_use_iterator consuming_use_end() const;

inline non_consuming_use_iterator non_consuming_use_begin() const;
inline non_consuming_use_iterator non_consuming_use_end() const;

/// Returns a range of all uses, which is useful for iterating over all uses.
/// To ignore debug-info instructions use swift::getNonDebugUses instead
/// (see comment in DebugUtils.h).
Expand All @@ -285,6 +296,12 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
/// and it has a single consuming user. Returns .none otherwise.
inline Operand *getSingleConsumingUse() const;

/// Returns a range of all consuming uses
inline consuming_use_range getConsumingUses() const;

/// Returns a range of all non consuming uses
inline non_consuming_use_range getNonConsumingUses() const;

template <class T>
inline T *getSingleUserOfType() const;

Expand Down Expand Up @@ -711,8 +728,10 @@ class Operand {
TheValue->FirstUse = this;
}

friend class ValueBase;
friend class ValueBaseUseIterator;
friend class ValueUseIterator;
friend class ConsumingUseIterator;
friend class NonConsumingUseIterator;
template <unsigned N> friend class FixedOperandList;
friend class TrailingOperandsList;
};
Expand All @@ -729,6 +748,7 @@ using OperandValueArrayRef = ArrayRefView<Operand, SILValue, getSILValueType>;
/// An iterator over all uses of a ValueBase.
class ValueBaseUseIterator : public std::iterator<std::forward_iterator_tag,
Operand*, ptrdiff_t> {
protected:
Operand *Cur;
public:
ValueBaseUseIterator() = default;
Expand Down Expand Up @@ -770,6 +790,74 @@ inline ValueBase::use_iterator ValueBase::use_end() const {
inline iterator_range<ValueBase::use_iterator> ValueBase::getUses() const {
return { use_begin(), use_end() };
}

class ConsumingUseIterator : public ValueBaseUseIterator {
public:
explicit ConsumingUseIterator(Operand *cur) : ValueBaseUseIterator(cur) {}
ConsumingUseIterator &operator++() {
assert(Cur && "incrementing past end()!");
assert(Cur->isConsumingUse());
while ((Cur = Cur->NextUse)) {
if (Cur->isConsumingUse())
break;
}
return *this;
}

ConsumingUseIterator operator++(int unused) {
ConsumingUseIterator copy = *this;
++*this;
return copy;
}
};

inline ValueBase::consuming_use_iterator
ValueBase::consuming_use_begin() const {
auto cur = FirstUse;
while (cur && !cur->isConsumingUse()) {
cur = cur->NextUse;
}
return ValueBase::consuming_use_iterator(cur);
}

inline ValueBase::consuming_use_iterator ValueBase::consuming_use_end() const {
return ValueBase::consuming_use_iterator(nullptr);
}

class NonConsumingUseIterator : public ValueBaseUseIterator {
public:
explicit NonConsumingUseIterator(Operand *cur) : ValueBaseUseIterator(cur) {}
NonConsumingUseIterator &operator++() {
assert(Cur && "incrementing past end()!");
assert(!Cur->isConsumingUse());
while ((Cur = Cur->NextUse)) {
if (!Cur->isConsumingUse())
break;
}
return *this;
}

NonConsumingUseIterator operator++(int unused) {
NonConsumingUseIterator copy = *this;
++*this;
return copy;
}
};

inline ValueBase::non_consuming_use_iterator
ValueBase::non_consuming_use_begin() const {
auto cur = FirstUse;
while (cur && cur->isConsumingUse()) {
cur = cur->NextUse;
}
return ValueBase::non_consuming_use_iterator(cur);
}

inline ValueBase::non_consuming_use_iterator
ValueBase::non_consuming_use_end() const {
return ValueBase::non_consuming_use_iterator(nullptr);
}

inline bool ValueBase::hasOneUse() const {
auto I = use_begin(), E = use_end();
if (I == E) return false;
Expand Down Expand Up @@ -806,6 +894,15 @@ inline Operand *ValueBase::getSingleConsumingUse() const {
return result;
}

inline ValueBase::consuming_use_range ValueBase::getConsumingUses() const {
return {consuming_use_begin(), consuming_use_end()};
}

inline ValueBase::non_consuming_use_range
ValueBase::getNonConsumingUses() const {
return {non_consuming_use_begin(), non_consuming_use_end()};
}

inline bool ValueBase::hasTwoUses() const {
auto iter = use_begin(), end = use_end();
for (unsigned i = 0; i < 2; ++i) {
Expand Down
61 changes: 12 additions & 49 deletions lib/SIL/Utils/OwnershipUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,15 @@ bool swift::isOwnershipForwardingInst(SILInstruction *i) {
return isOwnershipForwardingValueKind(SILNodeKind(i->getKind()));
}

bool swift::isReborrowInstruction(const SILInstruction *i) {
switch (i->getKind()) {
case SILInstructionKind::BranchInst:
return true;
default:
return false;
}
}

//===----------------------------------------------------------------------===//
// Borrowing Operand
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -163,7 +172,7 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
return os;
}

void BorrowingOperand::visitEndScopeInstructions(
void BorrowingOperand::visitLocalEndScopeInstructions(
function_ref<void(Operand *)> func) const {
switch (kind) {
case BorrowingOperandKind::BeginBorrow:
Expand All @@ -181,18 +190,8 @@ void BorrowingOperand::visitEndScopeInstructions(
return;
}
case BorrowingOperandKind::Branch:
for (auto *succBlock :
cast<BranchInst>(op->getUser())->getSuccessorBlocks()) {
auto *arg = succBlock->getArgument(op->getOperandNumber());
for (auto *use : arg->getUses()) {
if (use->isConsumingUse()) {
func(use);
}
}
}
return;
}
llvm_unreachable("Covered switch isn't covered");
}

void BorrowingOperand::visitBorrowIntroducingUserResults(
Expand Down Expand Up @@ -267,46 +266,10 @@ void BorrowingOperand::visitUserResultConsumingUses(
}
}

bool BorrowingOperand::getImplicitUses(
void BorrowingOperand::getImplicitUses(
SmallVectorImpl<Operand *> &foundUses,
std::function<void(Operand *)> *errorFunction) const {
if (!areAnyUserResultsBorrowIntroducers()) {
visitEndScopeInstructions([&](Operand *op) { foundUses.push_back(op); });
return false;
}

// Ok, we have an instruction that introduces a new borrow scope and its
// result is that borrow scope. In such a case, we need to not just add the
// end scope instructions of this scoped operand, but also look through any
// guaranteed phis and add their end_borrow to this list as well.
SmallVector<BorrowingOperand, 8> worklist;
SmallPtrSet<Operand *, 8> visitedValue;
worklist.push_back(*this);
visitedValue.insert(op);
bool foundError = false;
while (!worklist.empty()) {
auto scopedOperand = worklist.pop_back_val();
scopedOperand.visitConsumingUsesOfBorrowIntroducingUserResults(
[&](Operand *op) {
if (auto subSub = BorrowingOperand::get(op)) {
if (!visitedValue.insert(op).second) {
if (errorFunction) {
(*errorFunction)(op);
}
foundError = true;
return;
}

worklist.push_back(*subSub);
visitedValue.insert(subSub->op);
return;
}

foundUses.push_back(op);
});
}

return foundError;
visitLocalEndScopeInstructions([&](Operand *op) { foundUses.push_back(op); });
}

//===----------------------------------------------------------------------===//
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/Verifier/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ target_sources(swiftSIL PRIVATE
LoadBorrowImmutabilityChecker.cpp
LinearLifetimeChecker.cpp
MemoryLifetime.cpp
ReborrowVerifier.cpp
SILOwnershipVerifier.cpp
SILVerifier.cpp)
12 changes: 8 additions & 4 deletions lib/SIL/Verifier/LinearLifetimeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,14 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse,
// must be instructions in the given block. Make sure that the non consuming
// user is strictly before the consuming user.
for (auto *nonConsumingUse : nonConsumingUsesInBlock) {
if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(),
[&nonConsumingUse](const SILInstruction &i) -> bool {
return nonConsumingUse->getUser() == &i;
}) == userBlock->end()) {
if (nonConsumingUse->getUser() != consumingUse->getUser()) {
if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(),
[&nonConsumingUse](const SILInstruction &i) -> bool {
return nonConsumingUse->getUser() == &i;
}) == userBlock->end()) {
continue;
}
} else if (isReborrowInstruction(consumingUse->getUser())) {
continue;
}

Expand Down
Loading