Skip to content

Commit

Permalink
[DestroyAddrHoisting] Don't fold for invalid paths
Browse files Browse the repository at this point in the history
If the address for which hoisting is being performed doesn't have an
invalid access path don't attempt to fold its destroy_addrs.

rdar://121486203
  • Loading branch information
nate-chandler committed Jan 25, 2024
1 parent 75ccbae commit 7219914
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 5 deletions.
3 changes: 2 additions & 1 deletion include/swift/SIL/MemAccessUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,8 @@ struct AccessPathWithBase {
// The "product leaves" are the leaves obtained by only looking through type
// products (structs and tuples) and NOT type sums (enums).
void visitProductLeafAccessPathNodes(
SILValue address, TypeExpansionContext tec, SILModule &module,
AccessPath rootPath, SILValue address, TypeExpansionContext tec,
SILModule &module,
std::function<void(AccessPath::PathNode, SILType)> visitor);

inline AccessPath AccessPath::compute(SILValue address) {
Expand Down
6 changes: 4 additions & 2 deletions lib/SIL/Utils/MemAccessUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1436,10 +1436,12 @@ AccessPathWithBase AccessPathWithBase::computeInScope(SILValue address) {
}

void swift::visitProductLeafAccessPathNodes(
SILValue address, TypeExpansionContext tec, SILModule &module,
AccessPath rootPath, SILValue address, TypeExpansionContext tec,
SILModule &module,
std::function<void(AccessPath::PathNode, SILType)> visitor) {
assert(rootPath.isValid());
assert(AccessPath::compute(address) == rootPath);
SmallVector<std::pair<SILType, IndexTrieNode *>, 32> worklist;
auto rootPath = AccessPath::compute(address);
auto *node = rootPath.getPathNode().node;
worklist.push_back({address->getType(), node});
while (!worklist.empty()) {
Expand Down
18 changes: 16 additions & 2 deletions lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,13 @@ bool HoistDestroys::rewriteDestroys(const AccessStorage &storage,
bool HoistDestroys::foldBarrier(SILInstruction *barrier,
const AccessStorage &storage,
const DeinitBarriers &deinitBarriers) {
auto rootPath = AccessPath::compute(storageRoot);
if (!rootPath.isValid()) {
// [invalid_access_path] The access path to storageRoot isn't understood.
// It can't be determined whether all of its leaves have been visited, so
// foldability can't be determined. Bail.
return false;
}

// The load [copy]s which will be folded into load [take]s if folding is
// possible.
Expand Down Expand Up @@ -592,7 +599,8 @@ bool HoistDestroys::foldBarrier(SILInstruction *barrier,
// it.
SmallPtrSet<AccessPath::PathNode, 16> trivialLeaves;

visitProductLeafAccessPathNodes(storageRoot, typeExpansionContext, module,
visitProductLeafAccessPathNodes(rootPath, storageRoot, typeExpansionContext,
module,
[&](AccessPath::PathNode node, SILType ty) {
if (ty.isTrivial(*function))
return;
Expand Down Expand Up @@ -744,10 +752,16 @@ bool HoistDestroys::checkFoldingBarrier(
// of the root storage which would be folded if folding were possible.
// Find its nontrivial product leaves and remove them from the set of
// leaves of the root storage which we're wating to see.
auto rootPath = AccessPath::compute(address);
// [invalid_access_path] The access path to storageRoot was understood, and
// address has identical storage to its storage. The access path to address
// must be valid.
assert(rootPath.isValid());

bool alreadySawLeaf = false;
bool alreadySawTrivialSubleaf = false;
visitProductLeafAccessPathNodes(
address, typeExpansionContext, module,
rootPath, address, typeExpansionContext, module,
[&](AccessPath::PathNode node, SILType ty) {
if (ty.isTrivial(*function)) {
bool inserted = !trivialLeaves.insert(node).second;
Expand Down
20 changes: 20 additions & 0 deletions test/SILOptimizer/hoist_destroy_addr.sil
Original file line number Diff line number Diff line change
Expand Up @@ -1179,3 +1179,23 @@ entry(%addr : $*MoE):
%retval = tuple ()
return %retval : $()
}

sil @getPointer : $@convention(thin) () -> Builtin.RawPointer

struct Nontrivial {
var guts: Builtin.AnyObject
}

sil [ossa] @rdar121327964 : $@convention(method) (@owned Nontrivial) -> () {
bb0(%0 : @owned $Nontrivial):
%6 = function_ref @getPointer : $@convention(thin) () -> Builtin.RawPointer
%7 = apply %6() : $@convention(thin) () -> Builtin.RawPointer
%8 = pointer_to_address %7 : $Builtin.RawPointer to [strict] $*Nontrivial
%9 = copy_value %0 : $Nontrivial
%10 = begin_access [modify] [dynamic] %8 : $*Nontrivial
store %9 to [assign] %10 : $*Nontrivial
end_access %10 : $*Nontrivial
destroy_value %0 : $Nontrivial
%14 = tuple ()
return %14 : $()
}

0 comments on commit 7219914

Please sign in to comment.