[Mlir-commits] [mlir] [mlir] Fix MemRefType alignment in ConvertVectorToLLVM (PR #137389)

[Lily Orth-Smith]

electriclilies wrote:

@banach_space Thanks for the response! I was thinking yesterday about the code I wrote here and I agree it is a bit ad-hoc and actually incorrect in some cases. 

If we consider this example from vector_to_llvm_interface.mlir, my implementation would try to calculate the alignment based on the total size of `memref<200x100xf32>`, since the vector.store op contains the type of the memref itself, not type of the memref slice. 
```
func.func @store_0d(%memref : memref<200x100xf32>, %i : index, %j : index) {
  %val = arith.constant dense<11.0> : vector<f32>
  vector.store %val, %memref[%i, %j] : memref<200x100xf32>, vector<f32>
  return
}
```
I think adding a flag is a solution that would work well for us. In our case, we always have the vector type specified, so I would prefer to use the vector type directly instead of extracting the data type and size from the memref type, converting that into a vector type, and then getting the alignment. 

> IIUC, this format is supported by LLVM’s data layout:

Yes, it is supported, but the vector alignments aren't actually being used by MLIR during vector lowering. So specifying the vector alignments doesn't actually cause the generated vectors to be aligned. 

For example, when we set the alignment of f32 scalars to 64 bits, the alignment in the code generated below is 8, even if we've set the alignment of 4096 vectors (128 * 32) in the data layout string.

`    llvm.store %41, %43 {alignment 8 =  i64} : vector<128xf32>, !llvm.ptr
`
In terms of how we are specifying the data layout -- we have a tablegen file representing our target which contains the data layout string. I think we probably set the data layout string in the LLVMContext somewhere, but I'm not very familiar with that code path. I will look into it and get back to you. 

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