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

    <tr>
        <th>Summary</th>
        <td>
            SelectionDAG store merging default implementations are unreasonable
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            good first issue,
            llvm:SelectionDAG
      </td>
    </tr>

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

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

<pre>
    The API for controlling store merging in SelectionDAG is bad, and the default implementations will infinite loop on any custom store legalization that decompose stores. This adds more work (and lost time debugging) for target maintainers, and should try to default to something safe.

https://github.com/llvm/llvm-project/blob/9bebf25ecbe6a8720dd581bd2a4f8d29aa763a42/llvm/include/llvm/CodeGen/TargetLowering.h#L681

https://github.com/llvm/llvm-project/blob/9bebf25ecbe6a8720dd581bd2a4f8d29aa763a42/llvm/include/llvm/CodeGen/TargetLowering.h#L676

If a backend custom lowers a store and decomposes it, the default implementation reporting it's just safe to merge will infinite loop the legalizer. At a minimum, one of these hooks should default to checking `isOperationLegal(ISD::STORE, MemVT)` instead of an unconditional true. This would be more conservatively correct.

Additionally, the API here is too simple. Especially after legalization, the register value type and the in-memory value types may be relevant for truncating stores, so these callbacks should probably have more parameters. This is another instance where SelectionDAG's expressiveness for memory operation legality is inadequate.

I tried switching the default to the sample isOperationLegal example above, and a manageable number of test cases break in various targets. To resolve this issue, those failures need to be investigated to see what useful cases were missed. If we can extend the default logic to handle them as they are, that would be ideal but backend specific implementations of these hooks may need to be updated.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJzUVcGS2zYM_Rr5golHpi3ZPvjgZrOZnUknnWand4iEJGYpUiVAO-7Xdyjbu_a2OfTYizWWSODh4eEBmW3niXZF9UtRPcwwSR_iDiOTH2ZNMKfdc0-w_-0J2hBBBy8xOGd9BywhEgwUu_zPevhGjrTY4B_2n8EyNGgK9RHQG5CewFCLyQnYYXQ0kBfMZxmO1jmwvrXeCoELYYTgAf0JdGIJwyWRow6d_Wu6BNKjgCEdhjEwnU_wHJ57y4DGMAz5yjHEFyjUJiNwgQXEDhlHk7qMuVDbqSjB2JHAgNYLWk-Rr7C5D8kZkHgCCa8FSAAOA0k_sYAtzYvyoSj3599eZORiuS_UY6EeOyt9auY6DIV6dO5wfXwYY_hOWgr12LjQFOpx21DTqop0QzVu1qo0ptosGqNw1W6M2iKu6yWu1Fsc67VLht5efAyGPpMv1OPzVNKXcKRofTfvC7X8Um8W_w-c6_oW51MLCA3qF_LmqgiXLzDgRRq5Va9iYLCS-_dzyUGkMUSZVCuFWjN8TyxTJ3Nvs6Lp31SZI15USHEOewGEwXo7pCEnDJ4gtPkUE_QhvPBVPzfC0T3pl5y5qEvLX0eKE6QvOWyhNk_fHnJLlvtvz19__5Sj_krDH8-F2hZ1CdazEJqcBT0kr4M3Nl9HBxITXQbgOCVt6DwEOnimeECxB3In0CFG0nIn2b25hnGnK3V54nuKlAdZQgCeOJzDJx5J23wSsBWKd3N5vRyps5w_HtAlAjmN9GoD1n8YaAjxdPORYcBTRhzJ0QG9nOcyJq9RXr1mGksOF4Y1Opdl8cryGEODjTtBj4dL7SNGHEgoXr0h24MP0lOcyESvCY5TmbfmNWmCfoyRmO2BPDFPgC64w7Vrl9rllONaj4b-TCj3dvAEEi0Z4KMVPTnGrTBlqgYYM7nwXhBAP84fsAkHupoSwoAeO8LGEfg0NBQn3RELaMwD0ETCl-zIB4w2JL44XCYhQCQO7kAgZz440blr2UdbtC5FYvBEJoNrcr8OxGI7lPMrpkwZCiSmNrlLymPmcLDMZObw1MIxN8gD_RB6Z_8udFbnQD164zIOGgA5P0-A8YIG5U3G1hA6aJK82sCkwdbqf-ySd_OXVXVTShpNrmI-M7ul2S63OKPdYr1YVVW1XFezftcuaqywXbVrXLS1qvWG6uVyUSvKJqdoZneqVKuyKhflVqmqmtdGk9musFps27Kul8WqpAGtm2erm4fYzSaKd9tyvVjNHDbkeNq1SnUhGGhtZLm2QRXqY6HU5JLL_b0iVd7OcTd5cpM6Llalsyz8lkisONrdLeH7Ff2zBYyRIPlIyMFnUc1SdLv_vB6mGjgviFzq3wEAAP__uAPmow">