[mlir][vector] Add transfer_op LoadToStore forwarding and deadStore optimizations

[ThomasRaoux]

Summary

Add transformation to be able to forward transfer_write into transfer_read operation and to be able to remove dead transfer_write when a transfer_write is overwritten before being read.

[aartbik]

mlir/include/mlir/Dialect/Vector/VectorTransforms.h

271

This analysis transfer ops .... -> I cannot parse this part, is something missing?

mlir/lib/Dialect/Vector/VectorTransferOpTransforms.cpp

28

Returns (capital R, possibly s form)

31

can even be a for-loop without body!

36

empty line after (if we enclose longer regions, I prefer more whitespace)

also for the closing }

56

period at end, otherwise starts new sentence
(or use comma)

86

typo: potentially

90

typoe: overwriting

114

does not need {}

156

typo: dominated

171

to many "it"s
It there is a write, but its memory if disjoint with ... we can ignore the write ....

mlir/test/Dialect/Vector/vector-transferop-opt.mlir

16

can you cleanup the layout of this file for 80-colums, so it reads a bit easier

[ThomasRaoux]

Address review comments.

[ThomasRaoux]

mlir/include/mlir/Dialect/Vector/VectorTransforms.h

271

I removed this sentence as it wasn't clear indeed and didn't add much.

[aartbik]

mlir/include/mlir/Dialect/Vector/VectorUtils.h

165

perhaps a bit nitpicky, but I find the isXXX easier to read than the plural areXXX
can't we keep it singular with e.g. isDisjointTransferSet or something like that?

mlir/lib/Dialect/Vector/VectorTransferOpTransforms.cpp

91

found such a ....?

100

can you make this cascade of if statements a if/elseif/elseif sequence
since very branch has a continue, I find that a bit easier to read and get the logic

223

this sentence is missing something

224

typo: opportunity

mlir/lib/Dialect/Vector/VectorUtils.cpp

331

sentence does not flow?

[ThomasRaoux]

Address review comments

[ThomasRaoux]

mlir/lib/Dialect/Vector/VectorTransferOpTransforms.cpp

100

I'm not sure there is a nice way to do that since for the second if not all the cases end with a continue. I replaces the second continue with an else, hopefully that makes it clearer.

mlir/lib/Dialect/Vector/VectorUtils.cpp

331

It was part of the code moved. I fixed the sentence based on what I think the original author meant.

[aartbik]

mlir/lib/Dialect/Vector/VectorTransferOpTransforms.cpp

40

last nit (but this is subjective so feel free to ignore). The "OpOpt" is a bit of a mouthful.
You could just call this TransferOptimization.

[ThomasRaoux]

Thanks Aart.

mlir/lib/Dialect/Vector/VectorTransferOpTransforms.cpp

40

Done.

mlir/include/mlir/Dialect/Vector/VectorTransforms.h

PointwiseExtractPattern(
       : OpRewritePattern<ExtractMapOp>(context), filter(constraint) {}
   LogicalResult matchAndRewrite(ExtractMapOp extract,
                                 PatternRewriter &rewriter) const override;
 
 private:
   FilterConstraintType filter;
 };
 
+/// Implements transfer op write to read forwarding and dead transfer write
+/// optimization. This analysis transfer ops to detect cases where we read a
+/// value written by previous transfer write allowing us to remove the transfer
+/// read.
+void transferOpflowOpt(FuncOp func);
+
 } // namespace vector
 
 //===----------------------------------------------------------------------===//
 // Finer-grained patterns exposed for more control over individual lowerings.
 //===----------------------------------------------------------------------===//
 
 /// Progressive lowering of a `vector.contract %a, %b, %c` with row-major matmul
 /// semantics to:

mlir/include/mlir/Dialect/Vector/VectorUtils.h

 class AffineForOp;
 class AffineMap;
 class Location;
 class MemRefType;
 class OpBuilder;
 class Operation;
 class Value;
 class VectorType;
+class VectorTransferOpInterface;
 
 /// Return the number of elements of basis, `0` if empty.
 int64_t computeMaxLinearIndex(ArrayRef<int64_t> basis);
 
 /// Given a shape with sizes greater than 0 along all dimensions,
 /// return the distance, in number of elements, between a slice in a dimension
 /// and the next slice in the same dimension.
 ///   e.g. shape[3, 4, 5] -> linearization_basis[20, 5, 1]
makePermutationMap(Operation *op, ArrayRef<Value> indices,
                    const DenseMap<Operation *, unsigned> &loopToVectorDim);
 
 /// Build the default minor identity map suitable for a vector transfer. This
 /// also handles the case memref<... x vector<...>> -> vector<...> in which the
 /// rank of the identity map must take the vector element type into account.
 AffineMap getTransferMinorIdentityMap(MemRefType memRefType,
                                       VectorType vectorType);
 
+/// Return true if we can prove that the transfer operations access disjoint
+/// memory.
+bool areTransferOpsDisjoint(VectorTransferOpInterface transferA,
+                            VectorTransferOpInterface transferB);
+
 namespace matcher {
 
 /// Matches vector.transfer_read, vector.transfer_write and ops that return a
 /// vector type that is a multiple of the sub-vector type. This allows passing
 /// over other smaller vector types in the function and avoids interfering with
 /// operations on those.
 /// This is a first approximation, it can easily be extended in the future.
 /// TODO: this could all be much simpler if we added a bit that a vector type to
 /// mark that a vector is a strict super-vector but it still does not warrant
 /// adding even 1 extra bit in the IR for now.
 bool operatesOnSuperVectorsOf(Operation &op, VectorType subVectorType);
 
 } // end namespace matcher
 } // end namespace mlir
 
 #endif // MLIR_DIALECT_VECTOR_VECTORUTILS_H_

mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp

 
 #include "mlir/Dialect/Linalg/Transforms/Hoisting.h"
 #include "mlir/Analysis/SliceAnalysis.h"
 #include "mlir/Dialect/Linalg/IR/LinalgOps.h"
 #include "mlir/Dialect/SCF/SCF.h"
 #include "mlir/Dialect/SCF/Utils.h"
 #include "mlir/Dialect/StandardOps/IR/Ops.h"
 #include "mlir/Dialect/Vector/VectorOps.h"
+#include "mlir/Dialect/Vector/VectorUtils.h"
 #include "mlir/IR/Dominance.h"
 #include "mlir/IR/Function.h"
 #include "mlir/Transforms/LoopUtils.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "linalg-hoisting"
 
func.walk([&changed](Operation *op) {
         loop.moveOutOfLoop({op});
       else // Move DeallocOp outside of the loop.
         op->moveAfter(loop);
       changed = true;
     });
   }
 }
 
-/// Return true if we can prove that the transfer operations access disjoint
-/// memory.
-static bool isDisjoint(VectorTransferOpInterface transferA,
-                       VectorTransferOpInterface transferB) {
-  if (transferA.memref() != transferB.memref())
-    return false;
-  // For simplicity only look at transfer of same type.
-  if (transferA.getVectorType() != transferB.getVectorType())
-    return false;
-  unsigned rankOffset = transferA.getLeadingMemRefRank();
-  for (unsigned i = 0, e = transferA.indices().size(); i < e; i++) {
-    auto indexA = transferA.indices()[i].getDefiningOp<ConstantOp>();
-    auto indexB = transferB.indices()[i].getDefiningOp<ConstantOp>();
-    // If any of the indices are dynamic we cannot prove anything.
-    if (!indexA || !indexB)
-      continue;
-
-    if (i < rankOffset) {
-      // For dimension used as index if we can prove that index are different we
-      // know we are accessing disjoint slices.
-      if (indexA.getValue().cast<IntegerAttr>().getInt() !=
-          indexB.getValue().cast<IntegerAttr>().getInt())
-        return true;
-    } else {
-      // For this dimension, we slice a part of the memref we need to make sure
-      // the intervals accessed don't overlap.
-      int64_t distance =
-          std::abs(indexA.getValue().cast<IntegerAttr>().getInt() -
-                   indexB.getValue().cast<IntegerAttr>().getInt());
-      if (distance >= transferA.getVectorType().getDimSize(i - rankOffset))
-        return true;
-    }
-  }
-  return false;
-}
-
 void mlir::linalg::hoistRedundantVectorTransfers(FuncOp func) {
   bool changed = true;
   while (changed) {
     changed = false;
 
     func.walk([&](vector::TransferReadOp transferRead) {
       LLVM_DEBUG(DBGS() << "Candidate for hoisting: "
                         << *transferRead.getOperation() << "\n");
func.walk([&](vector::TransferReadOp transferRead) {
       for (auto &use : transferRead.memref().getUses()) {
         if (!dom.properlyDominates(loop, use.getOwner()))
           continue;
         if (use.getOwner() == transferRead.getOperation() ||
             use.getOwner() == transferWrite.getOperation())
           continue;
         if (auto transferWriteUse =
                 dyn_cast<vector::TransferWriteOp>(use.getOwner())) {
-          if (!isDisjoint(
+          if (!areTransferOpsDisjoint(
                   cast<VectorTransferOpInterface>(transferWrite.getOperation()),
                   cast<VectorTransferOpInterface>(
                       transferWriteUse.getOperation())))
             return WalkResult::advance();
         } else if (auto transferReadUse =
                        dyn_cast<vector::TransferReadOp>(use.getOwner())) {
-          if (!isDisjoint(
+          if (!areTransferOpsDisjoint(
                   cast<VectorTransferOpInterface>(transferWrite.getOperation()),
                   cast<VectorTransferOpInterface>(
                       transferReadUse.getOperation())))
             return WalkResult::advance();
         } else {
           // Unknown use, we cannot prove that it doesn't alias with the
           // transferRead/transferWrite operations.
           return WalkResult::advance();

mlir/lib/Dialect/Vector/CMakeLists.txt

 add_mlir_dialect_library(MLIRVector
   VectorOps.cpp
+  VectorTransferOpTransforms.cpp
   VectorTransforms.cpp
   VectorUtils.cpp
   EDSC/Builders.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/Vector
 
   DEPENDS

mlir/lib/Dialect/Vector/VectorTransferOpTransforms.cpp

+//===- VectorTransferOpTransforms.cpp - transfer op transforms ------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements functions concerned with optimizing transfer_read and
+// transfer_write ops.
+//
+//===----------------------------------------------------------------------===//
+#include "mlir/Dialect/StandardOps/IR/Ops.h"
+#include "mlir/Dialect/Vector/VectorOps.h"
+#include "mlir/Dialect/Vector/VectorTransforms.h"
+#include "mlir/Dialect/Vector/VectorUtils.h"
+#include "mlir/IR/Dominance.h"
+#include "mlir/IR/Function.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Debug.h"
+
+#define DEBUG_TYPE "vector-transfer-opt"
+
+#define DBGS() (llvm::dbgs() << '[' << DEBUG_TYPE << "] ")
+
+using namespace mlir;
+
+/// return the ancestor op in the region or nullptr if the region is not
+/// an ancestor of the op.
+static Operation *findAncestorOpInRegion(Region *region, Operation *op) {
+  while (op != nullptr && op->getParentRegion() != region)
+    op = op->getParentOp();
+  return op;
+}
+
+namespace {
+class TransferOpOptimization {
+public:
+  TransferOpOptimization(FuncOp func)
+      : dominators(func), postDominators(func) {}
+  void deadStoreOp(vector::TransferWriteOp);
+  void storeToLoadForwarding(vector::TransferReadOp);
+  void removeDeadOp() {
+    for (Operation *op : opToErase)
+      op->erase();
+    opToErase.clear();
+  }
+
+private:
+  bool isReachable(Operation *start, Operation *dest);
+  DominanceInfo dominators;
+  PostDominanceInfo postDominators;
+  std::vector<Operation *> opToErase;
+};
+
+/// Return true if there is a path from start operation to dest operation
+/// otherwise return false. The operations have to be in the same region.
+bool TransferOpOptimization::isReachable(Operation *start, Operation *dest) {
+  assert(start->getParentRegion() == dest->getParentRegion() &&
+         "This function only works for ops i the same region");
+  // Simple case where the start op dominate the destination.
+  if (dominators.dominates(start, dest))
+    return true;
+  Block *startBlock = start->getBlock();
+  Block *destBlock = dest->getBlock();
+  SmallVector<Block *, 32> worklist(startBlock->succ_begin(),
+                                    startBlock->succ_end());
+  SmallPtrSet<Block *, 32> visited;
+  while (!worklist.empty()) {
+    Block *bb = worklist.pop_back_val();
+    if (!visited.insert(bb).second)
+      continue;
+    if (dominators.dominates(bb, destBlock))
+      return true;
+    worklist.append(bb->succ_begin(), bb->succ_end());
+  }
+  return false;
+}
+
+/// For transfer_write to overwrite fully another transfer_write must:
+/// 1. Access the same memref with the same indices and vector type.
+/// 2. Post-dominate the other transfer_write operation.
+/// If several candidates are available, one must be post-dominated by all the
+/// others since they are all post-dominating the same transfer_write. We only
+/// consider the transfer_write post-dominated by all the other candidates as
+/// this will be the first transfer_write executed after the potetially dead
+/// transfer_write.
+/// If we found such a  we know that the original transfer_write is
+/// dead if all reads that can be reached from the potentially dead
+/// transfer_write are dominated by the overwritting transfer_write.
+void TransferOpOptimization::deadStoreOp(vector::TransferWriteOp write) {
+  LLVM_DEBUG(DBGS() << "Candidate for dead store: " << *write.getOperation()
+                    << "\n");
+  llvm::SmallVector<Operation *, 8> reads;
+  Operation *firstOverwriteCandidate = nullptr;
+  for (auto *user : write.memref().getUsers()) {
+    if (user == write.getOperation())
+      continue;
+    if (auto nextWrite = dyn_cast<vector::TransferWriteOp>(user)) {
+      // Check candidate that can override the store.
+      if (write.indices() == nextWrite.indices() &&
+          write.getVectorType() == nextWrite.getVectorType() &&
+          write.permutation_map() == write.permutation_map() &&
+          postDominators.postDominates(nextWrite, write)) {
+        if (firstOverwriteCandidate == nullptr ||
+            postDominators.postDominates(firstOverwriteCandidate, nextWrite))
+          firstOverwriteCandidate = nextWrite;
+        else
+          assert(
+              postDominators.postDominates(nextWrite, firstOverwriteCandidate));
+      }
+      continue;
+    }
+    if (auto read = dyn_cast<vector::TransferReadOp>(user)) {
+      // Don't need to consider disjoint reads.
+      if (areTransferOpsDisjoint(
+              cast<VectorTransferOpInterface>(write.getOperation()),
+              cast<VectorTransferOpInterface>(read.getOperation())))
+        continue;
+    }
+    reads.push_back(user);
+  }
+  if (firstOverwriteCandidate == nullptr)
+    return;
+  Region *topRegion = firstOverwriteCandidate->getParentRegion();
+  Operation *writeAncestor = findAncestorOpInRegion(topRegion, write);
+  assert(writeAncestor &&
+         "write op should be recursively part of the top region");
+
+  for (Operation *read : reads) {
+    Operation *readAncestor = findAncestorOpInRegion(topRegion, read);
+    // TODO: if the read and write have the same ancestor we could recurse in
+    // the region to know if the read is reachable with more precision.
+    if (readAncestor == nullptr || !isReachable(writeAncestor, readAncestor))
+      continue;
+    if (!dominators.dominates(firstOverwriteCandidate, read)) {
+      LLVM_DEBUG(DBGS() << "Store may not be dead due to op: " << *read
+                        << "\n");
+      return;
+    }
+  }
+  LLVM_DEBUG(DBGS() << "Found dead store: " << *write.getOperation()
+                    << " overwritten by: " << *firstOverwriteCandidate << "\n");
+  opToErase.push_back(write.getOperation());
+}
+
+/// A transfer_write candidate to storeToLoad forwarding must:
+/// 1. Access the same memref with the same indices and vector type as the
+/// transfer_read.
+/// 2. Dominate the transfer_read operation.
+/// If several candidates are available, one must be dominated by all the others
+/// since they are all dominating the same transfer_read. We only consider the
+/// transfer_write dominated by all the other candidates as this will be the
+/// last transfer_write executed before the transfer_read.
+/// If we found such a candidate we can do the forwarding if all the other
+/// potentially aliasing ops that may reach the transfer_read are post-domiated
+/// by the transfer_write.
+void TransferOpOptimization::storeToLoadForwarding(
+    vector::TransferReadOp read) {
+  if (read.hasMaskedDim())
+    return;
+  LLVM_DEBUG(DBGS() << "Candidate for Forwarding: " << *read.getOperation()
+                    << "\n");
+  SmallVector<Operation *, 8> blockingWrites;
+  vector::TransferWriteOp lastwrite = nullptr;
+  for (Operation *user : read.memref().getUsers()) {
+    if (isa<vector::TransferReadOp>(user))
+      continue;
+    if (auto write = dyn_cast<vector::TransferWriteOp>(user)) {
+      // If there is a write but we can prove that it is disjoint we can ignore
+      // it.
+      if (areTransferOpsDisjoint(
+              cast<VectorTransferOpInterface>(write.getOperation()),
+              cast<VectorTransferOpInterface>(read.getOperation())))
+        continue;
+      if (dominators.dominates(write, read) && !write.hasMaskedDim() &&
+          write.indices() == read.indices() &&
+          write.getVectorType() == read.getVectorType() &&
+          write.permutation_map() == read.permutation_map()) {
+        if (lastwrite == nullptr || dominators.dominates(lastwrite, write))
+          lastwrite = write;
+        else
+          assert(dominators.dominates(write, lastwrite));
+        continue;
+      }
+    }
+    blockingWrites.push_back(user);
+  }
+
+  if (lastwrite == nullptr)
+    return;
+
+  Region *topRegion = lastwrite.getParentRegion();
+  Operation *readAncestor = findAncestorOpInRegion(topRegion, read);
+  assert(readAncestor &&
+         "read op should be recursively part of the top region");
+
+  for (Operation *write : blockingWrites) {
+    Operation *writeAncestor = findAncestorOpInRegion(topRegion, write);
+    // TODO: if the store and read have the same ancestor we could recurse in
+    // the region to know if the read is reachable with more precision.
+    if (writeAncestor == nullptr || !isReachable(writeAncestor, readAncestor))
+      continue;
+    if (!postDominators.postDominates(lastwrite, write)) {
+      LLVM_DEBUG(DBGS() << "Fail to do write to read forwarding due to op: "
+                        << *write << "\n");
+      return;
+    }
+  }
+
+  LLVM_DEBUG(DBGS() << "Forward value from " << *lastwrite.getOperation()
+                    << " to: " << *read.getOperation() << "\n");
+  read.replaceAllUsesWith(lastwrite.vector());
+  opToErase.push_back(read.getOperation());
+}
+} // namespace
+
+void mlir::vector::transferOpflowOpt(FuncOp func) {
+  TransferOpOptimization opt(func);
+  // Run store to load forwarding first has it can expose more dead store
+  // oportuinity.
+  func.walk(
+      [&](vector::TransferReadOp read) { opt.storeToLoadForwarding(read); });
+  opt.removeDeadOp();
+  func.walk([&](vector::TransferWriteOp write) { opt.deadStoreOp(write); });
+  opt.removeDeadOp();
+}

mlir/lib/Dialect/Vector/VectorUtils.cpp

bool matcher::operatesOnSuperVectorsOf(Operation &op,
   // between parallel, reduction and possibly other cases.
   if (!ratio.hasValue()) {
     return false;
   }
 
   return true;
 }
 
+bool mlir::areTransferOpsDisjoint(VectorTransferOpInterface transferA,
+                                  VectorTransferOpInterface transferB) {
+  if (transferA.memref() != transferB.memref())
+    return false;
+  // For simplicity only look at transfer of same type.
+  if (transferA.getVectorType() != transferB.getVectorType())
+    return false;
+  unsigned rankOffset = transferA.getLeadingMemRefRank();
+  for (unsigned i = 0, e = transferA.indices().size(); i < e; i++) {
+    auto indexA = transferA.indices()[i].getDefiningOp<ConstantOp>();
+    auto indexB = transferB.indices()[i].getDefiningOp<ConstantOp>();
+    // If any of the indices are dynamic we cannot prove anything.
+    if (!indexA || !indexB)
+      continue;
+
+    if (i < rankOffset) {
+      // For dimension used as index if we can prove that index are different we
+      // know we are accessing disjoint slices.
+      if (indexA.getValue().cast<IntegerAttr>().getInt() !=
+          indexB.getValue().cast<IntegerAttr>().getInt())
+        return true;
+    } else {
+      // For this dimension, we slice a part of the memref we need to make sure
+      // the intervals accessed don't overlap.
+      int64_t distance =
+          std::abs(indexA.getValue().cast<IntegerAttr>().getInt() -
+                   indexB.getValue().cast<IntegerAttr>().getInt());
+      if (distance >= transferA.getVectorType().getDimSize(i - rankOffset))
+        return true;
+    }
+  }
+  return false;
+}

mlir/test/Dialect/Vector/vector-transferop-opt.mlir

+// RUN: mlir-opt %s -test-vector-transferop-opt | FileCheck %s
+
+// CHECK-LABEL: func @forward_dead_store
+//   CHECK-NOT:   vector.transfer_write
+//   CHECK-NOT:   vector.transfer_read
+//       CHECK:   scf.for
+//       CHECK:   }
+//       CHECK:   vector.transfer_write
+//       CHECK:   return
+func @forward_dead_store(%arg0: i1, %arg1 : memref<4x4xf32>, %v0 : vector<1x4xf32>,
+  %v1 : vector<1x4xf32>, %i : index) {
+  %c1 = constant 1 : index
+  %c4 = constant 4 : index
+  %c0 = constant 0 : index
+  %cf0 = constant 0.0 : f32
+  vector.transfer_write %v0, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  %0 = vector.transfer_read %arg1[%c1, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+  %x = scf.for %i0 = %c0 to %c4 step %c1 iter_args(%acc = %0) -> (vector<1x4xf32>) {
+    %1 = addf %acc, %acc : vector<1x4xf32>
+    scf.yield %1 : vector<1x4xf32>
+  }
+  vector.transfer_write %x, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  return
+}
+
+// CHECK-LABEL: func @forward_nested
+//       CHECK:   vector.transfer_write
+//       CHECK:   vector.transfer_write
+//       CHECK:   scf.if
+//   CHECK-NOT:     vector.transfer_read
+//       CHECK:   }
+//       CHECK:   vector.transfer_write
+//       CHECK:   return
+func @forward_nested(%arg0: i1, %arg1 : memref<4x4xf32>, %v0 : vector<1x4xf32>,
+  %v1 : vector<1x4xf32>, %i : index) {
+  %c0 = constant 0 : index
+  %c1 = constant 1 : index
+  %cf0 = constant 0.0 : f32
+  vector.transfer_write %v1, %arg1[%i, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  vector.transfer_write %v0, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  %x = scf.if %arg0 -> (vector<1x4xf32>) {
+    %0 = vector.transfer_read %arg1[%c1, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+    scf.yield %0 : vector<1x4xf32>
+  } else {
+    scf.yield %v1 : vector<1x4xf32>
+  }
+  vector.transfer_write %x, %arg1[%c0, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  return
+}
+
+// Negative test, the transfer_write in the scf.if region block the store to
+// load forwarding because we don't recursively look into the region to realize
+// that the transfer_write cannot reach the transfer_read.
+// CHECK-LABEL: func @forward_nested_negative
+//       CHECK:   vector.transfer_write
+//       CHECK:   scf.if
+//       CHECK:     vector.transfer_read
+//       CHECK:   } else {
+//       CHECK:     vector.transfer_write
+//       CHECK:   }
+//       CHECK:   vector.transfer_write
+//       CHECK:   return
+func @forward_nested_negative(%arg0: i1, %arg1 : memref<4x4xf32>, %v0 : vector<1x4xf32>,
+  %v1 : vector<1x4xf32>, %i : index) {
+  %c0 = constant 0 : index
+  %c1 = constant 1 : index
+  %cf0 = constant 0.0 : f32
+  vector.transfer_write %v0, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  %x = scf.if %arg0 -> (vector<1x4xf32>) {
+    %0 = vector.transfer_read %arg1[%c1, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+    scf.yield %0 : vector<1x4xf32>
+  } else {
+    vector.transfer_write %v1, %arg1[%i, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+    scf.yield %v1 : vector<1x4xf32>
+  }
+  vector.transfer_write %x, %arg1[%c0, %i] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  return
+}
+
+// CHECK-LABEL: func @dead_store_region
+//       CHECK:   vector.transfer_write
+//       CHECK:   scf.if
+//       CHECK:   } else {
+//       CHECK:     vector.transfer_read
+//       CHECK:   }
+//       CHECK:   scf.if
+//   CHECK-NOT:     vector.transfer_write
+//       CHECK:   }
+//       CHECK:   vector.transfer_write
+//   CHECK-NOT:   vector.transfer_write
+//       CHECK:   vector.transfer_read
+//       CHECK:   return
+func @dead_store_region(%arg0: i1, %arg1 : memref<4x4xf32>, %v0 : vector<1x4xf32>,
+  %v1 : vector<1x4xf32>, %i : index) -> (vector<1x4xf32>) {
+  %c0 = constant 0 : index
+  %c1 = constant 1 : index
+  %cf0 = constant 0.0 : f32
+  vector.transfer_write %v0, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  %x = scf.if %arg0 -> (vector<1x4xf32>) {
+    scf.yield %v1 : vector<1x4xf32>
+  } else {
+    %0 = vector.transfer_read %arg1[%i, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+    scf.yield %0 : vector<1x4xf32>
+  }
+  scf.if %arg0 {
+    vector.transfer_write %v0, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  }
+  vector.transfer_write %x, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  vector.transfer_write %x, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  %1 = vector.transfer_read %arg1[%i, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+  return %1 : vector<1x4xf32>
+}
+
+// CHECK-LABEL: func @dead_store_negative
+//       CHECK:   scf.if
+//       CHECK:     vector.transfer_write
+//       CHECK:     vector.transfer_read
+//       CHECK:   } else {
+//       CHECK:   }
+//       CHECK:   vector.transfer_write
+//       CHECK:   return
+func @dead_store_negative(%arg0: i1, %arg1 : memref<4x4xf32>, %v0 : vector<1x4xf32>,
+  %v1 : vector<1x4xf32>, %i : index) {
+  %c0 = constant 0 : index
+  %c1 = constant 1 : index
+  %cf0 = constant 0.0 : f32
+  %x = scf.if %arg0 -> (vector<1x4xf32>) {
+    vector.transfer_write %v0, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+    %0 = vector.transfer_read %arg1[%i, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+    scf.yield %0 : vector<1x4xf32>
+  } else {
+    scf.yield %v1 : vector<1x4xf32>
+  }
+  vector.transfer_write %x, %arg1[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  return
+}
+
+// CHECK-LABEL: func @dead_store_nested_region
+//       CHECK:   scf.if
+//       CHECK:     vector.transfer_read
+//       CHECK:     scf.if
+//   CHECK-NOT:       vector.transfer_write
+//       CHECK:     }
+//       CHECK:     vector.transfer_write
+//       CHECK:   }
+//       CHECK:   return
+func @dead_store_nested_region(%arg0: i1, %arg1: i1, %arg2 : memref<4x4xf32>, %v0 : vector<1x4xf32>,
+  %v1 : vector<1x4xf32>, %i : index) {
+  %c0 = constant 0 : index
+  %c1 = constant 1 : index
+  %cf0 = constant 0.0 : f32
+  scf.if %arg0 {
+    %0 = vector.transfer_read %arg2[%i, %c0], %cf0 {masked = [false, false]} : memref<4x4xf32>, vector<1x4xf32>
+    scf.if %arg1 {
+      vector.transfer_write %v1, %arg2[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+    }
+    vector.transfer_write %v0, %arg2[%c1, %c0] {masked = [false, false]} : vector<1x4xf32>, memref<4x4xf32>
+  }
+  return
+}
+

mlir/test/lib/Transforms/TestVectorTransforms.cpp

if (useLinalgOps)
       options.setVectorTransferSplit(VectorTransferSplit::LinalgCopy);
     else
       options.setVectorTransferSplit(VectorTransferSplit::VectorTransfer);
     patterns.insert<VectorTransferFullPartialRewriter>(ctx, options);
     applyPatternsAndFoldGreedily(getFunction(), std::move(patterns));
   }
 };
 
+struct TestVectorTransferOpt
+    : public PassWrapper<TestVectorTransferOpt, FunctionPass> {
+  void runOnFunction() override { transferOpflowOpt(getFunction()); }
+};
+
 } // end anonymous namespace
 
 namespace mlir {
 namespace test {
 void registerTestVectorConversions() {
   PassRegistration<TestVectorToVectorConversion> vectorToVectorPass(
       "test-vector-to-vector-conversion",
       "Test conversion patterns between ops in the vector dialect");
PassRegistration<TestVectorTransferFullPartialSplitPatterns>
                                      "transfer ops via scf.if + linalg ops");
   PassRegistration<TestVectorDistributePatterns> distributePass(
       "test-vector-distribute-patterns",
       "Test conversion patterns to distribute vector ops in the vector "
       "dialect");
   PassRegistration<TestVectorToLoopPatterns> vectorToForLoop(
       "test-vector-to-forloop",
       "Test conversion patterns to break up a vector op into a for loop");
+  PassRegistration<TestVectorTransferOpt> transferOpOpt(
+      "test-vector-transferop-opt",
+      "Test optimization transformations for transfer ops");
 }
 } // namespace test
 } // namespace mlir