[Mlir-commits] [mlir] [mlir][linalg] Prevent hoisting of transfer pairs in the presence of aliases (PR #145235)

[Andrzej Warzyński]

================
@@ -312,14 +312,27 @@ void mlir::linalg::hoistRedundantVectorTransfers(Operation *root,
           transferRead.getPermutationMap() != transferWrite.getPermutationMap())
         return WalkResult::advance();
 
+      // Check 2. for xfer_read
       auto *source = transferRead.getBase().getDefiningOp();
       if (source && isa_and_nonnull<ViewLikeOpInterface>(source))
         return WalkResult::advance();
 
+      auto base = transferRead.getBase();
+      for (auto *user : base.getUsers())
+        if (isa_and_nonnull<ViewLikeOpInterface>(user))
----------------
banach-space wrote:

Using `isDisjointTransferSet`only  makes sense when the underlying `base` is identical, see:  https://github.com/llvm/llvm-project/blob/577199f9221ebc805a69372a2b19f4c8ebaf1daf/mlir/lib/Dialect/Vector/IR/VectorOps.cpp#L318-L319

Looking at the example from your repro, that would indeed apply to the pair that you want to hoist.

```mlir
    %assume_align = memref.assume_alignment %alloc, 64 : memref<4096x4096xf16>
    scf.for %arg0 = %c256 to %c4096 step %c256 {
      %0 = vector.transfer_read %assume_align[%c0, %c0], %cst {in_bounds = [true, true]} : memref<4096x4096xf16>, vector<16x16xf16>
      %1 = vector.transfer_read %alloc_0[%arg0, %arg0], %cst {in_bounds = [true, true]} : memref<4096x4096xf16>, vector<16x16xf16>
      %2 = vector.contract {indexing_maps = [#map, #map1, #map2], iterator_types = ["parallel", "parallel", "reduction"], kind = #vector.kind<add>} %1, %1, %0 : vector<16x16xf16>, vector<16x16xf16> into vector<16x16xf16>
      vector.transfer_write %2, %assume_align[%c0, %c0] {in_bounds = [true, true]} : vector<16x16xf16>, memref<4096x4096xf16>
    }
```

However, it won't work if we modify the test a bit:

```mlir
    %assume_align = memref.assume_alignment %alloc, 64 : memref<4096x4096xf16>
    scf.for %arg0 = %c256 to %c4096 step %c256 {
      %0 = vector.transfer_read %assume_align[%c0, %c0], %cst {in_bounds = [true, true]} : memref<4096x4096xf16>, vector<16x16xf16>
      %1 = vector.transfer_read %alloc_0[%arg0, %arg0], %cst {in_bounds = [true, true]} : memref<4096x4096xf16>, vector<16x16xf16>
      %2 = vector.contract {indexing_maps = [#map, #map1, #map2], iterator_types = ["parallel", "parallel", "reduction"], kind = #vector.kind<add>} %1, %1, %0 : vector<16x16xf16>, vector<16x16xf16> into vector<16x16xf16>
      vector.transfer_write %2, %alloc[%c0, %c0] {in_bounds = [true, true]} : vector<16x16xf16>, memref<4096x4096xf16>
    }
```
In this case, the base for `xfer_read` and `xfer_write` Ops will be different (`%assume_align` and `%alloc`, respectively).

https://github.com/llvm/llvm-project/pull/145235