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

    <tr>
        <th>Summary</th>
        <td>
            [mlir] clean-up memref.collapse_shape with dynamic dimension of size 1
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            new issue,
            mlir
      </td>
    </tr>

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

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

<pre>
    The lowering of `memref.collapse_shape` may return the wrong stride if some of the collapsed dimensions are dynamic at compile time and `1` at runtime.

The issue stems from the fact that dimensions of size `1` are supposed to be skipped  (see the comment below from 
`computeCollapsedLayoutMap`) when computing the resulting stride of the collapsed dimensions, but we don't generate the code that does that for dynamic dimensions.

This was a deliberate choice when refactoring the lowering of `memref.collapse_shape` (see 64f99842a6c03fdb57349fcaed3f4821ef612ed1 and https://reviews.llvm.org/D136483#3909856) because:
- Supporting that would greatly pessimism codegen: to compute the stride of a collapsed dimension we would need to filter out the size-1 dimensions from the reassociation group and take the `min` of the resulting set of strides (i.e., a bunch of branchy code), and more importantly
- Dimension of size 1 where this kind of gymnastics are required are arguably not truly contiguous (and thus fall into UB as described in the documentation of `memref.collpase_shape`)

Anyhow, we need to decide if non-contiguous dimensions of size 1 are allowed and either fix the expansion of `memref.collapse_shape` (in `expand-strided-metadata`) or document clearly that they are not supported and suggest an alternative (in particular to front-end writers).

Note: Static non-contiguous dimensions of size 1 are currently correctly expanded, but this is a band aid to the bigger problem (i.e., we saw them happen in practice). If we decide that dimensions of size 1 shouldn't be used in `memref.collapse_shape` we should get rid of that code too. 

---
Comment in `computeCollapsedLayoutMap`:
```
The result stride of a reassociation group is the stride of the last entry
of the reassociation. (...) Dimensions of size 1 should be skipped, because
their strides are meaningless and could have any arbitrary value
```
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJyMVl1v27gS_TXyy8CGTNmK9eCHNEGAC9x7X7r7vBiRI4lbilT5EdX99YuhZMdt026BIJEicebMmTNnhCHo3hKdi-OH4vi8wRQH58-fpTNubCluWqcu5z8GAuNm8tr24Doo6nKk0VO3k84YnAL9FQacqKhLGPECnmLyFuJAMHtnewjRa0WgOwhuJA7Bz66HFSg9kg3a2QDoCdTF4qglYATpxkkbgqhHArSKc-85D0bwyfK_d0X5XJSPy2-GqkNIBCHSGKDzbszJOpQR4oDxPpnrIOiv9BbUE4Q0TY5BRQctQfikp4kUQCFOgWgFPo5kI7Rk3LykWDHUJQNOkZ6utf0XLy7F_-FU1GUhGpgHsrC8xGxyOE8hmXy38vQLfgrxBG2KMBMoZwvxEKEnSx7jFZqitUxHYbnqnL9R-hbpO9p0gBkDICgyul0CysFpSQtkT8yg81fQv6uHlbb60DXN6SCwlmXVqfb4UB2aTiKpqjucxJ66ei9I7XOPhxinUFSPhXgpxIunV01z2BnzOu6c7wvx8ryv6sOpKkRVNWVzOtbMbEsSUyA-l2vawkdupV95xgizS0ZB7wmjucBEIehRhzGT1pMtqkdu-trBXOVbQ_C9dnAblqCWFsV02kTy4FJczuuvtN3fS-4mSE8YgpMaIwfqvUtTLj7ipyU3s6otc7jq4U4nFLN2M7rAHOsd7VgbCG2ycuCnrUcrh0uurhBNfmoVjM4T6JGJQRvN5UrW862o61TsufOewegAn7RV_KS_jBZD1HIZVU-fk_ak8g36PmFrLmBdhOiT4eQ26j65lFHm-oYUoENjQNvo4M8PgAEUBel1Swr04hvKycQztrDzg8YmvNMYF3en5Ud7GdzM5c5064siuTqQdXZ7h-odN9gvxRiWuMqckY4Deej0lwyOvkx4o-pfxK8tv5FPqO3SMLUdKaLCiKsn8Hyu9YI0hN5cFsHGgS4ZDBMaFjWvkELqewoR0AKy5CxG_Uprxgl91DIZ9FmT3tm4Jatg9jqSD4Vovpn-_7vIYwMfmW752wzJ5D2xhEA670ny1VIoqatPZe1o9pWWUaPO3WASW9335GHyrjU03mt4Jgg480sjDDhNZFkVk0cZtWQp7-A_XXbApak_c_U9hIGHc7HJliCFRWC_ahknHxafoAheq2X68iriXM7t4J677Xa7XDytS2GJ_8s1cHUovl5-brtrmfFvfOc9o9DhO3vKlowhAtno15m-2cbd-R0TvdvtWHbPP6fsbvPlTq7WmsPGgbS_eQ8LYSS02vaGQsjalDnEgK-8slnArY4e_QVe0ST6rvaNOleqqRrc0HlfPzycjnV5EpvhrIQ4NEIe9niqq4pabE5NW1GlHjrRnbpmo8-iFFVZldX-WJ5Ksauolg8lCnmqDrLuVHEoaURtbqtjkz8NzvV-fzxtDLZkQv7yEcLSvHw3FEIU4qkQYjTa883xeePPHGDbpj4Uh9LoEN-20SbqaPL3Uz5wfM4TbLdpgnc1BrOOw4_r-K0Dm-TN-dsN2Os4pHYn3ViIF068_tlO3v1NMhbiJWMPhXjJtf0TAAD__2g4Xds">