Merge pull request #71157 from nate-chandler/nfc/20240125/1/tweak-visit-product-leaves

NFC: [MemAccessUtils] Tweaked API.

Author: nate-chandler

include/swift/SIL/MemAccessUtils.h

@@ -1271,9 +1271,10 @@ 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(
-    AccessPath rootPath, SILValue address, TypeExpansionContext tec,
-    SILModule &module,
+//
+// Returns false if the access path couldn't be computed.
+bool visitProductLeafAccessPathNodes(
+    SILValue address, TypeExpansionContext tec, SILModule &module,
     std::function<void(AccessPath::PathNode, SILType)> visitor);
 
 inline AccessPath AccessPath::compute(SILValue address) {

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1435,12 +1435,13 @@ AccessPathWithBase AccessPathWithBase::computeInScope(SILValue address) {
       .findAccessPath(address);
 }
 
-void swift::visitProductLeafAccessPathNodes(
-    AccessPath rootPath, SILValue address, TypeExpansionContext tec,
-    SILModule &module,
+bool swift::visitProductLeafAccessPathNodes(
+    SILValue address, TypeExpansionContext tec, SILModule &module,
     std::function<void(AccessPath::PathNode, SILType)> visitor) {
-  assert(rootPath.isValid());
-  assert(AccessPath::compute(address) == rootPath);
+  auto rootPath = AccessPath::compute(address);
+  if (!rootPath.isValid()) {
+    return false;
+  }
   SmallVector<std::pair<SILType, IndexTrieNode *>, 32> worklist;
   auto *node = rootPath.getPathNode().node;
   worklist.push_back({address->getType(), node});
@@ -1474,6 +1475,7 @@ void swift::visitProductLeafAccessPathNodes(
       visitor(AccessPath::PathNode(node), silType);
     }
   }
+  return true;
 }
 
 void AccessPath::Index::print(raw_ostream &os) const {

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

@@ -560,14 +560,6 @@ 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.
   llvm::SmallVector<LoadInst *, 4> loads;
@@ -599,13 +591,19 @@ bool HoistDestroys::foldBarrier(SILInstruction *barrier,
   // it.
   SmallPtrSet<AccessPath::PathNode, 16> trivialLeaves;
 
-  visitProductLeafAccessPathNodes(rootPath, storageRoot, typeExpansionContext,
-                                  module,
-                                  [&](AccessPath::PathNode node, SILType ty) {
-                                    if (ty.isTrivial(*function))
-                                      return;
-                                    leaves.insert(node);
-                                  });
+  bool succeeded = visitProductLeafAccessPathNodes(
+      storageRoot, typeExpansionContext, module,
+      [&](AccessPath::PathNode node, SILType ty) {
+        if (ty.isTrivial(*function))
+          return;
+        leaves.insert(node);
+      });
+  if (!succeeded) {
+    // [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;
+  }
 
   for (auto *instruction = barrier; instruction != nullptr;
        instruction = instruction->getPreviousInstruction()) {
@@ -752,16 +750,10 @@ 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(
-        rootPath, address, typeExpansionContext, module,
+    auto succeeded = visitProductLeafAccessPathNodes(
+        address, typeExpansionContext, module,
         [&](AccessPath::PathNode node, SILType ty) {
           if (ty.isTrivial(*function)) {
             bool inserted = !trivialLeaves.insert(node).second;
@@ -771,6 +763,11 @@ bool HoistDestroys::checkFoldingBarrier(
           bool erased = leaves.erase(node);
           alreadySawLeaf = alreadySawLeaf || !erased;
         });
+    (void)succeeded;
+    // [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(succeeded);
     if (alreadySawLeaf) {
       // We saw this non-trivial product leaf already.  That means there are
       // multiple load [copy]s or copy_addrs of at least one product leaf