Vector transfers hoisting and memrefs subviews

Hi all,
I am running into a strange behaviour from the hoistRedundantVectorTransfers pass (in mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp) where from my understanding the aliasing analysis is incorrect and the hoisting may change the semantics of a program. I would however like to confirm with someone more knowledgeable than me if it is really the case…

Here is the smallest piece of code I can come up with where the issue arises:

#map0 = affine_map<(d0, d1)[s0, s1] -> (d0 * s1 + s0 + d1)>
#map1 = affine_map<(d0, d1) -> (d0 * 32 + d1 * 2)>
module {
  func @myfunc(%arg0: index, %arg1: memref<16x32xf32, #map0>) {
    %cst = arith.constant 0.000000e+00 : f32
    %c32 = arith.constant 32 : index
    %c0 = arith.constant 0 : index
    %0 = memref.alloc() {alignment = 128 : i64} : memref<16x32xf32>
    %1 = memref.subview %0[0, 0] [16, 16] [1, 2] : memref<16x32xf32> to memref<16x16xf32, #map1>
    %2 = vector.transfer_read %arg1[%c0, %c0], %cst : memref<16x32xf32, #map0>, vector<16x32xf32>
    scf.for %arg2 = %c0 to %arg0 step %c32 {
      vector.transfer_write %2, %0[%c0, %c0] {in_bounds = [true, true]} : vector<16x32xf32>, memref<16x32xf32>
      %3 = vector.transfer_read %1[%c0, %c0], %cst {in_bounds = [true, true]} : memref<16x16xf32, #map1>, vector<16x16xf32>
      %4 = arith.addf %3, %3 : vector<16x16xf32>
      vector.transfer_write %4, %1[%c0, %c0] {in_bounds = [true, true]} : vector<16x16xf32>, memref<16x16xf32, #map1>
      memref.copy %0, %arg1 : memref<16x32xf32> to memref<16x32xf32, #map0>
    }
    memref.dealloc %0 : memref<16x32xf32>
    return
  }
}

When running mlir-opt -test-linalg-hoisting=test-hoist-redundant-transfers on this code, the following is generated:

#map0 = affine_map<(d0, d1)[s0, s1] -> (d0 * s1 + s0 + d1)>
#map1 = affine_map<(d0, d1) -> (d0 * 32 + d1 * 2)>
module {
  func @myfunc(%arg0: index, %arg1: memref<16x32xf32, #map0>) {
    %cst = arith.constant 0.000000e+00 : f32
    %c32 = arith.constant 32 : index
    %c0 = arith.constant 0 : index
    %0 = memref.alloc() {alignment = 128 : i64} : memref<16x32xf32>
    %1 = memref.subview %0[0, 0] [16, 16] [1, 2] : memref<16x32xf32> to memref<16x16xf32, #map1>
    %2 = vector.transfer_read %arg1[%c0, %c0], %cst : memref<16x32xf32, #map0>, vector<16x32xf32>
    %3 = vector.transfer_read %1[%c0, %c0], %cst {in_bounds = [true, true]} : memref<16x16xf32, #map1>, vector<16x16xf32>
    %4 = scf.for %arg2 = %c0 to %arg0 step %c32 iter_args(%arg3 = %3) -> (vector<16x16xf32>) {
      vector.transfer_write %2, %0[%c0, %c0] {in_bounds = [true, true]} : vector<16x32xf32>, memref<16x32xf32>
      %5 = arith.addf %arg3, %arg3 : vector<16x16xf32>
      memref.copy %0, %arg1 : memref<16x32xf32> to memref<16x32xf32, #map0>
      scf.yield %5 : vector<16x16xf32>
    }
    vector.transfer_write %4, %1[%c0, %c0] {in_bounds = [true, true]} : vector<16x16xf32>, memref<16x16xf32, #map1>
    memref.dealloc %0 : memref<16x32xf32>
    return
  }
}

As you can see the pass has hoisted both the transfer_read and the transfer_write operations that are acting on the %1 memref. I am pretty sure this changes the semantics of the program because now the %3 vector comes from reading %0 (through the subview %1) just after its allocation (so it will contain junk values), while before it was done after the transfer_write of %2 (which took elements from %arg1) into %0.

On top of this, by hoisting the transfer_write the %arg1 parameter is not updated with the value of %5 anymore, and after returning the function, nothing has changed since the result of the arith.addf operation has only been put into the local %0 memref (again through a subview) after the copy into %arg1 has been done.

In essence, the generated code does not do anything substantial anymore since it just reads from %arg1 into a buffer and then writes that buffer back into %arg1, before updating the buffer for nothing because it deallocates it just after.

My understanding is that this is wrong and is due to the aliasing analysis in the hoisting pass not taking into account that any operations reading/writing into a subview of a memref should also be considered a dependency, instead of just keeping track of the “direct” uses of that memref. Am I wrong?

The totally probable alternative is that I don’t properly understand the semantics of subviews and that any operation done on a subview of a memref should not be considered as being also done on the original memref, is that so?

Thanks for the help

EDIT: Replaced example with something slightly simpler.

It looks like the code implicitly assumes subviews are folded via --fold-memref-subview-ops. That might be why it is just a test pass. @ThomasRaoux

Yes this is a bug, the code should be checking that there isn’t a subview of the memref that would cause aliasing.
I can help fixing this if you need, let me know.

Thank you very much for confirming my suspicions!

I already have a patch on the way for this, but if you could review it when I’m done I would appreciate it a lot

1 Like