<table border="1" cellspacing="0" cellpadding="8">
    <tr>
        <th>Issue</th>
        <td>
            <a href=https://github.com/llvm/llvm-project/issues/59896>59896</a>
        </td>
    </tr>

    <tr>
        <th>Summary</th>
        <td>
            [MLIR] expand-strided-metadata's expansions are invalid
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            mlir
      </td>
    </tr>

    <tr>
      <th>Assignees</th>
      <td>
      </td>
    </tr>

    <tr>
      <th>Reporter</th>
      <td>
          qcolombet
      </td>
    </tr>
</table>

<pre>
    The `expand-strided-metadata` pass breaks down complicated `memref` operations by modeling their effects on the offset, sizes, and strides directly and reassembling them together.

The issue at hand is the reassembling part is done with `memref.reinterpret_cast` and this is not technically correct.

E.g.,
```
res = memref.subview(src)
```
=>
```
base, offset, sizes, strides = memref.extract_strided_metadata(src)
final_sizes = subSizes
final_strides = <some math> strides
final_offset = <some math> offset
res = reinterpret_cast base to final_offset, final_sizes, final_strides <-- that part is not technically correct
```

The problem is that `memref.reinterpret_cast` doesn't technically create a `memref` that is composed of its arguments. It only reinterprets `base` as being a `memref` with the given arguments.
(See https://discourse.llvm.org/t/rfc-what-is-the-intended-semantic-of-memref-reinterpret-cast/67259 for the details)

The difference is subtle, but essentially, the problem is `memref.reinterpret_cast` is used to make the type system happy, not to assemble a memref, i.e., the resulting codegen could just be noop and that would be fine.
Put differently, before the expansion, `res` points to the subview of `src`. After the the expansion, `res` points to the base buffer of src.

We didn't notice the issue earlier because the current LLVM codegen does what we expect (`res.offset = final_offset`) but it could have done a noop (or`assert(res.offset == final_offset)`).

I believe to fix this issue we would need to introduce a new `memref` operation that would build a memref from its arguments. 
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJyMVt1u4zYTfRr6ZiBBoW3ZvvDFfskaWGAX-NAt2suAIkcWdylS5YzsdZ--IKU4dpptCwRJzJ8zZ86cGVMR2aNH3Iv1_8T6aaFG7kLc_6GDC32DvGiCuex_7RBEXeGPQXlTEEdr0BQ9sjKKlagrGBQRNBHVdwITzh506AdntWI06WqPfcQ2nQwDRsU2eILmAn0w6Kw_AndoI2DbomaC4NMChLYlZCEfgeyfSOkf5Q1MBAiMjajZXfJiREWEffOC1gOHI3KHsRTVk6g-TL9TKpZoRFAMXbpoKce6uz-oyGnDBI9wtty95lBGtJ4xDhH5WSvilFTC4c5SuuMDA6PuvNXKuQvoEBPNOxYfy2Mp5OO8VFfzT_4YkUAsn2AOR2NzsngWcktRC7l7945YPonlx3e3GkWYhHtHyxcdb6LhD45K8_Nc4-drje_Dt9Yr95yB8m0am68Z9Xb3Bl0sHyn0CL3iTiw_vkS-PT7xe_f0TP1OnbdVgJQncIBbtJTkDdWbj1duj0UB3Cm-lvwn5Xtf9qulhhgah_1kJsX_YhcTkLyQmzeRIipGUPf9kuEs5YYKhAZCC5YJVDyOPXqmEj4xBO8ut5pQAsmlT-4kaDDZ-g10Nnby_tGe0N8gztnJ7VdE6JgHEssPQh6EPBhLOoyRsHTu1JchHoU8sJCH2Ori3CkuLBXcYZGo-DQmCHvl2eoitMUUu7ghWmRN5KHeyPUO2hAzIYOsrKNXu1-FNrZtMaLXqY2T79hlezcjAxKhZ5vETEt8X5d_LoklGJO6HKBX3zFf5suAQBdi7KFTw5BRsz8CzMMilWsWVD6CLbF8iRyRRsdJdR0MHjGNxNEZ-DYmsyL4EIZ5biiGc95rMPkTZ_3_P_I1XZ5SarANcSKXhzHZ4NO6qKuIlAdxsJ4pMUyH5uGRPCPqKnVwXZXwoWWcdP7PQLm7mjGRSWAU9d08-z0VxkyW9oGtnqCnQYsqOosRGtRqpGlHjzFlBZ8___blKlBqCzhnOTIt1AxCbidO5c2AuOvxuhJyl-tveda4UyecZreadBZyG6Koq1S1yEJu7wH_hil3E-xdkp-gQWfxNI-ZHy8TP-V4xrmEHicTWc8xmFFnCnj-yXfgXfFH68zVTtDG0L9t9IXZL81uuVML3D_Um6XcrNZytej2ZlNVVVPVm2pba416IzetXpvdg35Yr6p2t7B7Wcll9VDtHlZyu1qVul5J3epVpVbrzW6rxarCXll3betFTmy_3m139cKpBh3lR4KUvbNRSJmeC3GfjhfNeCSxqpwlplcAtuzyw-LL50-_iPUT_Oz9IDf06sKUMIL1J-WsWYzR7e8H0NFyNzalDr2QhxRr_lMMMXxLg1oeMnMS8pDJ_xUAAP__fLT2-w">