Skip to content

[DRAFT][ownership] Test stdlib with ownership + Andy patches #35942

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions include/swift/SIL/SILArgument.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ class SILPhiArgument : public SILArgument {
/// this will be guaranteed to return a valid SILValue.
SILValue getIncomingPhiValue(SILBasicBlock *predBlock) const;

/// If this argument is a true phi, return the operand in the \p predBLock
/// associated with an incoming value.
///
/// \returns the operand or nullptr if this is not a true phi.
Operand *getIncomingPhiOperand(SILBasicBlock *predBlock) const;

/// If this argument is a phi, populate `OutArray` with the incoming phi
/// values for each predecessor BB. If this argument is not a phi, return
/// false.
Expand Down
71 changes: 68 additions & 3 deletions include/swift/SILOptimizer/Utils/CanonicalOSSALifetime.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@

namespace swift {

/// Convert this struct_extract into a copy+destructure. Return the destructured
/// result or invalid SILValue. The caller must delete the extract and its
/// now-dead copy use.
///
// If a copied-def is a struct-extract, attempt a destructure conversion
// %extract = struct_extract %... : $TypeWithSingleOwnershipValue
// %copy = copy_value %extract : $OwnershipValue
// To:
// %copy = copy_value %extract : $TypeWithSingleOwnershipValue
// (%extracted,...) = destructure %copy : $TypeWithSingleOwnershipValue
SILValue convertExtractToDestructure(StructExtractInst *extract);

/// Information about consumes on the extended-lifetime boundary. Consuming uses
/// within the lifetime are not included--they will consume a copy after
/// rewriting. For borrowed def values, the consumes do not include the end of
Expand Down Expand Up @@ -175,6 +187,50 @@ class CanonicalOSSAConsumeInfo {
SWIFT_ASSERT_ONLY_DECL(void dump() const LLVM_ATTRIBUTE_USED);
};

// Worklist of pointer-like things that have an invalid default value. Avoid
// revisiting nodes--suitable for DAGs, but pops finished nodes without
// preserving them in the vector.
//
// The primary API has two methods: intialize() and pop(). Others are provided
// for flexibility.
//
// TODO: make this a better utility.
template <typename T, unsigned SmallSize> struct PtrWorklist {
SmallPtrSet<T, SmallSize> ptrVisited;
SmallVector<T, SmallSize> ptrVector;

PtrWorklist() = default;

PtrWorklist(const PtrWorklist &) = delete;

void initialize(T t) {
clear();
insert(t);
}

template <typename R> void initializeRange(R &&range) {
clear();
ptrVisited.insert(range.begin(), range.end());
ptrVector.append(range.begin(), range.end());
}

T pop() { return empty() ? T() : ptrVector.pop_back_val(); }

bool empty() const { return ptrVector.empty(); }

unsigned size() const { return ptrVector.size(); }

void clear() {
ptrVector.clear();
ptrVisited.clear();
}

void insert(T t) {
if (ptrVisited.insert(t).second)
ptrVector.push_back(t);
}
};

/// Canonicalize OSSA lifetimes.
///
/// Allows the allocation of analysis state to be reused across calls to
Expand Down Expand Up @@ -221,11 +277,11 @@ class CanonicalizeOSSALifetime {
/// outisde the pruned liveness at the time it is discovered.
llvm::SmallPtrSet<DebugValueInst *, 8> debugValues;

/// Reuse a general worklist for def-use traversal.
SmallSetVector<SILValue, 8> defUseWorklist;
/// Reuse a general visited set for def-use traversal.
PtrWorklist<SILValue, 8> defUseWorklist;

/// Reuse a general worklist for CFG traversal.
SmallSetVector<SILBasicBlock *, 8> blockWorklist;
PtrWorklist<SILBasicBlock *, 8> blockWorklist;

/// Pruned liveness for the extended live range including copies. For this
/// purpose, only consuming instructions are considered "lifetime
Expand Down Expand Up @@ -298,6 +354,15 @@ class CanonicalizeOSSALifetime {

bool consolidateBorrowScope();

bool findBorrowScopeUses(llvm::SmallPtrSetImpl<SILInstruction *> &useInsts);

void filterOuterBorrowUseInsts(
llvm::SmallPtrSetImpl<SILInstruction *> &outerUseInsts);

void rewriteOuterBorrowUsesAndFindConsumes(
SILValue incomingValue,
llvm::SmallPtrSetImpl<SILInstruction *> &outerUseInsts);

bool computeCanonicalLiveness();

bool endsAccessOverlappingPrunedBoundary(SILInstruction *inst);
Expand Down
6 changes: 6 additions & 0 deletions lib/SIL/IR/SILArgument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ bool SILPhiArgument::getIncomingPhiValues(
return true;
}

Operand *SILPhiArgument::getIncomingPhiOperand(SILBasicBlock *predBlock) const {
if (!isPhiArgument())
return nullptr;
return getIncomingPhiOperandForPred(getParent(), predBlock, getIndex());
}

bool SILPhiArgument::getIncomingPhiOperands(
SmallVectorImpl<Operand *> &returnedPhiOperands) const {
if (!isPhiArgument())
Expand Down
11 changes: 10 additions & 1 deletion lib/SIL/Verifier/MemoryLifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ static bool canHandleAllocStack(AllocStackInst *asi) {
if (asi->hasDynamicLifetime())
return false;

SILType stackType = asi->getType();

// Currently in this verifier, we stop verifying if we find a switch_enum_addr
// use. This creates a problem since no one has gone through and changed the
// frontend/optimizer to understand that it needs to insert destroy_addr on
Expand All @@ -155,9 +157,16 @@ static bool canHandleAllocStack(AllocStackInst *asi) {
// implemented.
//
// https://bugs.swift.org/browse/SR-14123
if (asi->getType().getEnumOrBoundGenericEnum())
if (stackType.getEnumOrBoundGenericEnum())
return false;

// Same for tuples that have an enum element. We are just working around this
// for now until the radar above is solved.
if (auto tt = stackType.getAs<TupleType>())
for (unsigned i : range(tt->getNumElements()))
if (stackType.getTupleElementType(i).getEnumOrBoundGenericEnum())
return false;

// Otherwise we can optimize!
return true;
}
Expand Down
35 changes: 26 additions & 9 deletions lib/SIL/Verifier/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,31 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
LoadBorrowImmutabilityAnalysis loadBorrowImmutabilityAnalysis;
bool SingleFunction = true;

/// A cache of the isOperandInValueUse check. When we process an operand, we
/// fix this for each of its uses.
llvm::DenseSet<std::pair<SILValue, const Operand *>> isOperandInValueUsesCache;

/// Check that this operand appears in the use-chain of the value it uses.
bool isOperandInValueUses(const Operand *operand) {
SILValue value = operand->get();

// First check the cache.
if (isOperandInValueUsesCache.contains({value, operand}))
return true;

// Otherwise, compute the value and initialize the cache for each of the
// operand's value uses.
bool foundUse = false;
for (auto *use : value->getUses()) {
if (use == operand) {
foundUse = true;
}
isOperandInValueUsesCache.insert({value, use});
}

return foundUse;
}

SILVerifier(const SILVerifier&) = delete;
void operator=(const SILVerifier&) = delete;
public:
Expand Down Expand Up @@ -1112,7 +1137,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {

require(operand.getUser() == I,
"instruction's operand's owner isn't the instruction");
require(isInValueUses(&operand), "operand value isn't used by operand");
require(isOperandInValueUses(&operand), "operand value isn't used by operand");

if (operand.isTypeDependent()) {
require(isa<SILInstruction>(I),
Expand Down Expand Up @@ -1311,14 +1336,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
});
}

/// Check that this operand appears in the use-chain of the value it uses.
static bool isInValueUses(const Operand *operand) {
for (auto use : operand->get()->getUses())
if (use == operand)
return true;
return false;
}

/// \return True if all of the users of the AllocStack instruction \p ASI are
/// inside the same basic block.
static bool isSingleBlockUsage(AllocStackInst *ASI, DominanceInfo *Dominance){
Expand Down
77 changes: 63 additions & 14 deletions lib/SILOptimizer/LoopTransforms/LoopRotate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ static void mapOperands(SILInstruction *inst,
static void updateSSAForUseOfValue(
SILSSAUpdater &updater, SmallVectorImpl<SILPhiArgument *> &insertedPhis,
const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
SILBasicBlock *Header, SILBasicBlock *EntryCheckBlock, SILValue Res) {
SILBasicBlock *Header, SILBasicBlock *EntryCheckBlock, SILValue Res,
SmallVectorImpl<std::pair<SILBasicBlock *, unsigned>>
&accumulatedAddressPhis) {
// Find the mapped instruction.
assert(valueMap.count(Res) && "Expected to find value in map!");
SILValue MappedValue = valueMap.find(Res)->second;
Expand Down Expand Up @@ -159,49 +161,96 @@ static void updateSSAForUseOfValue(
&& "The entry check block should dominate the header");
updater.rewriteUse(*use);
}
// Canonicalize inserted phis to avoid extra BB Args.

// Canonicalize inserted phis to avoid extra BB Args and if we find an address
// phi, stash it so we can handle it after we are done rewriting.
bool hasOwnership = Header->getParent()->hasOwnership();
for (SILPhiArgument *arg : insertedPhis) {
if (SILValue inst = replaceBBArgWithCast(arg)) {
arg->replaceAllUsesWith(inst);
// DCE+SimplifyCFG runs as a post-pass cleanup.
// DCE replaces dead arg values with undef.
// SimplifyCFG deletes the dead BB arg.
continue;
}

// If we didn't simplify and have an address phi, stash the value so we can
// fix it up.
if (hasOwnership && arg->getType().isAddress())
accumulatedAddressPhis.emplace_back(arg->getParent(), arg->getIndex());
}
}

static void
updateSSAForUseOfInst(SILSSAUpdater &updater,
SmallVectorImpl<SILPhiArgument *> &insertedPhis,
const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
SILBasicBlock *header, SILBasicBlock *entryCheckBlock,
SILInstruction *inst) {
static void updateSSAForUseOfInst(
SILSSAUpdater &updater, SmallVectorImpl<SILPhiArgument *> &insertedPhis,
const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
SILBasicBlock *header, SILBasicBlock *entryCheckBlock, SILInstruction *inst,
SmallVectorImpl<std::pair<SILBasicBlock *, unsigned>>
&accumulatedAddressPhis) {
for (auto result : inst->getResults())
updateSSAForUseOfValue(updater, insertedPhis, valueMap, header,
entryCheckBlock, result);
entryCheckBlock, result, accumulatedAddressPhis);
}

/// Rewrite the code we just created in the preheader and update SSA form.
static void rewriteNewLoopEntryCheckBlock(
SILBasicBlock *header, SILBasicBlock *entryCheckBlock,
const llvm::DenseMap<ValueBase *, SILValue> &valueMap) {
SmallVector<SILPhiArgument *, 4> insertedPhis;
SmallVector<std::pair<SILBasicBlock *, unsigned>, 8> accumulatedAddressPhis;
SmallVector<SILPhiArgument *, 8> insertedPhis;
SILSSAUpdater updater(&insertedPhis);

// Fix PHIs (incoming arguments).
for (auto *arg : header->getArguments())
// Fix PHIs (incoming arguments). We iterate by index in case we replace the
// phi argument so we do not invalidate iterators.
for (unsigned i : range(header->getNumArguments())) {
auto *arg = header->getArguments()[i];
updateSSAForUseOfValue(updater, insertedPhis, valueMap, header,
entryCheckBlock, arg);
entryCheckBlock, arg, accumulatedAddressPhis);
}

auto instIter = header->begin();

// The terminator might change from under us.
while (instIter != header->end()) {
auto &inst = *instIter;
updateSSAForUseOfInst(updater, insertedPhis, valueMap, header,
entryCheckBlock, &inst);
entryCheckBlock, &inst, accumulatedAddressPhis);
++instIter;
}

// Then see if any of our phis were address phis. In such a case, rewrite the
// address to be a smuggled through raw pointer. We do this late to
// conservatively not interfere with the previous code's invariants.
//
// We also translate the phis into a BasicBlock, Index form so we are careful
// with invalidation issues around branches/args.
auto rawPointerTy =
SILType::getRawPointerType(header->getParent()->getASTContext());
auto rawPointerUndef = SILUndef::get(rawPointerTy, header->getModule());
auto loc = RegularLocation::getAutoGeneratedLocation();
while (!accumulatedAddressPhis.empty()) {
SILBasicBlock *block;
unsigned argIndex;
std::tie(block, argIndex) = accumulatedAddressPhis.pop_back_val();
auto *arg = cast<SILPhiArgument>(block->getArgument(argIndex));
assert(arg->getType().isAddress() && "Not an address phi?!");
for (auto *predBlock : block->getPredecessorBlocks()) {
Operand *predUse = arg->getIncomingPhiOperand(predBlock);
SILBuilderWithScope builder(predUse->getUser());
auto *newIncomingValue =
builder.createAddressToPointer(loc, predUse->get(), rawPointerTy);
predUse->set(newIncomingValue);
}
SILBuilderWithScope builder(arg->getNextInstruction());
SILType oldArgType = arg->getType();
auto *phiShim = builder.createPointerToAddress(
loc, rawPointerUndef, oldArgType, true /*isStrict*/,
false /*is invariant*/);
arg->replaceAllUsesWith(phiShim);
SILArgument *newArg = block->replacePhiArgument(
argIndex, rawPointerTy, OwnershipKind::None, nullptr);
phiShim->setOperand(newArg);
}
}

/// Update the dominator tree after rotating the loop.
Expand Down
Loading