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

    <tr>
        <th>Summary</th>
        <td>
            [mlir] Bugs in `loop-invariant-subset-hoisting` implementation
        </td>
    </tr>

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

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

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

<pre>
    I was reviewing existing implementation of `loop-invariant-subset-hoisting` and founds a couple issues. They are related to the [last change](https://github.com/llvm/llvm-project/pull/70623) from ~2years ago dealing with nested loops: 

- In [this function](https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L242), the `collectHoistableOps` parameter is not used. This means that the if you enumerate LoopLikeOps in pre-order and  apply the transformation `mlir::hoistLoopInvariantSubsets`, you can produce invalid IR since it may try to hoist args not belonging to the current loop at [this point](https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L332). 

- When checking use-def chain in the body of  the loop and there is a nested loop user in the use-def chain, at [this point](https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L268) there is no `if(nextValue) return failure()` like in the insertion user branch, which looks very suspicious. I wasn't able to create a failing test case. However since user order is not deterministic, it doesn't make sense for the check to be omitted.


</pre>
<img width="1" height="1" alt="" src="http://email.email.llvm.org/o/eJzMVU9v47YT_TT0ZWBDpizZPuiQbGBsgPzwA9ptex6RI4kNRQr846wv_ezFUHHa3Nqe9kRLFmfmzXtvBmM0oyPqRPMomqcN5jT50KkpmJj8MlHoMdGm9_rWPcMbRgh0NfRm3Aj03cTEP8y8WJrJJUzGO_ADiLay3i9b464YDLq0jbmPlLaTX--ItgJ0GgafnY6AoHxeLIGJMVPcwbeJboCBIJDFRBqShzQRiObRYkygJnQjieZJyNOU0hJF_SDkRcjLaNKU-53ys5AXa6_3Y7sE_zupJORlydYKeTlWrayFPMMQ_Ax_yBthiICjB01oGdibSRM4ilwA4-EsIKoHUT1s4dlxNWkyEYbsFEP_j_X01vdCXmY0jg9rAn9j-N23gC4OPsxRyMsvyVg-X7xfnu-d_eI1_c9z9vL3Ti2LkPWLPEghz0J-WdvWVspbSyp9ZQKwt_T_JTIJCwacKVEAE8H5BDmS5v6bCDOhi5AmTCWIGeDmM5DLMwVMBFzHi3nlUGAcLIG2PmgKhVnAZbG3cjHdQaz6EG1VMNYPon4ogvgE6OeiFC6Oq-eMCjm411kRsKKs0fD8E0Tj-EWCGW-Qwo01UsIBhnEF05P1bmQq3_WjcgjkUmETMH0wuHjj0g9EX10zfbsPsf02kQM1kXplMDnSVtPALjCOW8_Q2KPsvfKw4nOaHwLbCvDvQuYI4X7xUzTu-Q_cF9me2LEfqJxnOZlByJOj7-lXtJn4g0ApBwcDGpsDCcm3WO7WvNIdt3GRQlFk6UYf0KmJ8b9NRk3cp9cIVwo3iDkuRhmf4w7KFHRCHhOwjVhYKhDbAUu6IjbiEYWRdvDVv9GVwrtYS6LVI-920-y92Tgei4qTmwTa03uGGV8JIrlIMPiwKphFwFl7Aj-blEjvVpVsdFfrc33GDXX7Y7Nvm6au2s3UneWxwWOtD0qd5CBPh_PQSK3lYa8kVVpuTCcr2VTH6rA_NU3V7ioajns6nbA56fZU9eJQ0YzG7pjinQ_jpgzqbn84Vud2Y7EnG8sOkXIlWvI2CV2RRJ_HKA6VNTHFvyIkk2zZO-VC8wSPeSxz5B8tj887Z5OD7f61QNdlI-TlHca1k38GAAD__-3RZxY">