Skip to content

Commit bb69df4

Browse files
committed
NFC: [MemAccessUtils] Tweaked API.
Previously, `visitProductLeafAccessPathNodes` required its caller to provide both an `AccessPath` `path` and an `SILValue` `address` which satisfied `path == AccessPath::compute(address)` to force the caller to handle the case of an invalid `AccessPath`. Now, instead, it computes the value itself and returns false if it's invalid. It could be tweaked to also return false if the provided lambda returned false but that would make the only currently extant callers less pleasant and also would not be sufficient in the case of caller who wanted to distinguish between an invalid `AccessPath` and a particular leaf visit returning false.
1 parent 2ae159b commit bb69df4

File tree

3 files changed

+31
-31
lines changed

3 files changed

+31
-31
lines changed

include/swift/SIL/MemAccessUtils.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,9 +1271,10 @@ struct AccessPathWithBase {
12711271
//
12721272
// The "product leaves" are the leaves obtained by only looking through type
12731273
// products (structs and tuples) and NOT type sums (enums).
1274-
void visitProductLeafAccessPathNodes(
1275-
AccessPath rootPath, SILValue address, TypeExpansionContext tec,
1276-
SILModule &module,
1274+
//
1275+
// Returns false if the access path couldn't be computed.
1276+
bool visitProductLeafAccessPathNodes(
1277+
SILValue address, TypeExpansionContext tec, SILModule &module,
12771278
std::function<void(AccessPath::PathNode, SILType)> visitor);
12781279

12791280
inline AccessPath AccessPath::compute(SILValue address) {

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,12 +1435,13 @@ AccessPathWithBase AccessPathWithBase::computeInScope(SILValue address) {
14351435
.findAccessPath(address);
14361436
}
14371437

1438-
void swift::visitProductLeafAccessPathNodes(
1439-
AccessPath rootPath, SILValue address, TypeExpansionContext tec,
1440-
SILModule &module,
1438+
bool swift::visitProductLeafAccessPathNodes(
1439+
SILValue address, TypeExpansionContext tec, SILModule &module,
14411440
std::function<void(AccessPath::PathNode, SILType)> visitor) {
1442-
assert(rootPath.isValid());
1443-
assert(AccessPath::compute(address) == rootPath);
1441+
auto rootPath = AccessPath::compute(address);
1442+
if (!rootPath.isValid()) {
1443+
return false;
1444+
}
14441445
SmallVector<std::pair<SILType, IndexTrieNode *>, 32> worklist;
14451446
auto *node = rootPath.getPathNode().node;
14461447
worklist.push_back({address->getType(), node});
@@ -1474,6 +1475,7 @@ void swift::visitProductLeafAccessPathNodes(
14741475
visitor(AccessPath::PathNode(node), silType);
14751476
}
14761477
}
1478+
return true;
14771479
}
14781480

14791481
void AccessPath::Index::print(raw_ostream &os) const {

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -560,14 +560,6 @@ bool HoistDestroys::rewriteDestroys(const AccessStorage &storage,
560560
bool HoistDestroys::foldBarrier(SILInstruction *barrier,
561561
const AccessStorage &storage,
562562
const DeinitBarriers &deinitBarriers) {
563-
auto rootPath = AccessPath::compute(storageRoot);
564-
if (!rootPath.isValid()) {
565-
// [invalid_access_path] The access path to storageRoot isn't understood.
566-
// It can't be determined whether all of its leaves have been visited, so
567-
// foldability can't be determined. Bail.
568-
return false;
569-
}
570-
571563
// The load [copy]s which will be folded into load [take]s if folding is
572564
// possible.
573565
llvm::SmallVector<LoadInst *, 4> loads;
@@ -599,13 +591,19 @@ bool HoistDestroys::foldBarrier(SILInstruction *barrier,
599591
// it.
600592
SmallPtrSet<AccessPath::PathNode, 16> trivialLeaves;
601593

602-
visitProductLeafAccessPathNodes(rootPath, storageRoot, typeExpansionContext,
603-
module,
604-
[&](AccessPath::PathNode node, SILType ty) {
605-
if (ty.isTrivial(*function))
606-
return;
607-
leaves.insert(node);
608-
});
594+
bool succeeded = visitProductLeafAccessPathNodes(
595+
storageRoot, typeExpansionContext, module,
596+
[&](AccessPath::PathNode node, SILType ty) {
597+
if (ty.isTrivial(*function))
598+
return;
599+
leaves.insert(node);
600+
});
601+
if (!succeeded) {
602+
// [invalid_access_path] The access path to storageRoot isn't understood.
603+
// It can't be determined whether all of its leaves have been visited, so
604+
// foldability can't be determined. Bail.
605+
return false;
606+
}
609607

610608
for (auto *instruction = barrier; instruction != nullptr;
611609
instruction = instruction->getPreviousInstruction()) {
@@ -752,16 +750,10 @@ bool HoistDestroys::checkFoldingBarrier(
752750
// of the root storage which would be folded if folding were possible.
753751
// Find its nontrivial product leaves and remove them from the set of
754752
// leaves of the root storage which we're wating to see.
755-
auto rootPath = AccessPath::compute(address);
756-
// [invalid_access_path] The access path to storageRoot was understood, and
757-
// address has identical storage to its storage. The access path to address
758-
// must be valid.
759-
assert(rootPath.isValid());
760-
761753
bool alreadySawLeaf = false;
762754
bool alreadySawTrivialSubleaf = false;
763-
visitProductLeafAccessPathNodes(
764-
rootPath, address, typeExpansionContext, module,
755+
auto succeeded = visitProductLeafAccessPathNodes(
756+
address, typeExpansionContext, module,
765757
[&](AccessPath::PathNode node, SILType ty) {
766758
if (ty.isTrivial(*function)) {
767759
bool inserted = !trivialLeaves.insert(node).second;
@@ -771,6 +763,11 @@ bool HoistDestroys::checkFoldingBarrier(
771763
bool erased = leaves.erase(node);
772764
alreadySawLeaf = alreadySawLeaf || !erased;
773765
});
766+
(void)succeeded;
767+
// [invalid_access_path] The access path to storageRoot was understood, and
768+
// address has identical storage to its storage. The access path to address
769+
// must be valid.
770+
assert(succeeded);
774771
if (alreadySawLeaf) {
775772
// We saw this non-trivial product leaf already. That means there are
776773
// multiple load [copy]s or copy_addrs of at least one product leaf

0 commit comments

Comments
 (0)