Skip to content

[DestroyAddrHoisting] Don't fold for invalid paths. #71138

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 1 commit into from
Jan 25, 2024
Merged
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
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 : $()
}