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

    <tr>
        <th>Summary</th>
        <td>
            foldICmpWithDominatingICmp selects 1st BranchInst for a value, with multiple BranchInst in DominatorCache
        </td>
    </tr>

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

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

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

<pre>
    Hello, I came across a test at: https://godbolt.org/z/E7ezGeE7K, which has multiple dominators for a value (m) and the function foldICmpWithDominatorICmp() selects the 1st dominator out of these.
Though looking at all dominators (for (BranchInst *BI : DC.conditionsFor(X)) { ) is better, than earlier cheap-check for single-predecessor (BasicBlock *DomBB = CmpBB->getSinglePredecessor();), but I think, we end up not choosing the immediate dominator.  
I am not sure if this is desired so checking.

For the test https://godbolt.org/z/E7ezGeE7K, with current logic, dominator-cache returns 2 BranchInsts. The dominator edge1 ('entry->while.cond') for 1st BranchInst (in entry block) in dominator-cache for 'm' is selected to dominate 'if.end13' (Cmp) and further transformation is performed based on this selection.
However, the 2nd BranchInst (in while.cond block) in dominator-cache for 'm', is not considered. The dominator edge1 ('while.cond -> if.end13') for 2nd BranchInst, seems to be an immediate dominator. 
When the same same test: https://godbolt.org/z/E863r8vcz, is observed on 17.0.01 (without BranchInst from dominator-cache), the single-predecessor was considered, which is also immediate dominator and thus 'cmp18' was inverted to 'cmp18.not'.

So thought of checking the scenario where multiple dominators exists for a value, would it make sense to have some more logic to select more relevant dominator ? 
I tried adding 17.0.01 approach to select single-predecessor, with the change at: https://github.com/pgodeq/llvm-project/commit/ed695182eae2ad8fdcb573663abe6d16704187c5), when multiple BranchInst exist. 
I am not sure if we need this logic or any other logic for handling multiple dominators, so thought of understanding the perspective. 
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJyUVs1y2zYQfhrosmOOCEoiddDBsqPG00tnkpn0CgJLAjUIsAAoNXn6zoKSLdtqJr3IFigs9vvZDxQxmt4h7th6z9aPCzEl7cNu7L3CvxetV993n9Faz_gDPIEUA4KQwccIAhLGBCKx6h50SmNk1T3jB8YPvVett6nwoWf88IPxw6caf_yGn-rfqc5JG6lBiwjDZJMZLYLyg3Ei-RCh8wEEHIWdEBhvBsa3IJyCpBG6yclkvIPOW_X0MIzfTNKPl720wHhDGyJalCnmTWVMr_XBTwl8Rw8iFmz5yJb3X7Wfeg3W-2fjehAJhLXXLTHeUFeMN_sgnNRPLiZg_H7_BAT-8aGQ3ilDncWDD4w3fzK-pT5YvQf6ayK0mBIGwp-0cIAiWIMBpEYx3kmN8jlDj8b1Fu_GgAolxng-V0Qj99bLZzr30Q_7PbDqER6Gcb-_Y9WnHtOXvPOP140zF6za52YeoJ0SPEHSxj1nGRDQKZhGcD6B1N7T2ZkyMwyojEhXwhQAM1tPIIa8I04BwRCVJhJAhdEEVBA9ZDjG9WeC58-DD7l4ts3_M4xJGuQUAroE1vdG0upLa3dSSI0QME3BReDwqlIs4Ku-QgGoeiwhM1OjS-E7kXfSxmLWkPGa5CIhyDZv5G6Mg7wFWhIiy-o-dDEbpR4Yr4mU2YioIPnLT8nWtekKdKqs6GeMN9m5s8-7KSSNAVIQLnY-DCI73kQYMdB3VNCKiAq8m7mfzzDenen-7E94vFgNgTv1Ecgr5F9FQ-VMnL3iXTQKA6qf0Xt1BJEMV5AvHL9tjU6IiEMksloE4W4bcUb5TaPL-CKFUv4gZ_1SGjWbKjRH-eOMybcRw3FmtKyLZbHMGMh2FBdX5HXBD-9JOk9XbuXj8J5EvOLrNf5MBGGjv4XwHHcTBU8th7EkNnMh444Yzm66PCucT4zXb0bti4eUQy1n3WUa5xYlOhGMh5PGgDcTGP8xMb0J4ty2n6wCk2AQzwgRXURqQ4sjQvQDwuADztNJ67Mp58WAFo_CXccwqw4veZKCQQVCKerxwr8Yx-CF1Fe1PrL7Eg6ETGrherx9IZmkp7aQfmD8MN9tjB-sPQ53Y_B_oUyMH6QfBkP_oNps12XDUSAXqumUbNd1tdlUosWNKjf1clU2tVyfhT-REV94vDJL5rH4r9g8ITgkKWmEZ9qy9N_B5_mfl0gELZyyxM0NsfLMvFF7cgpDTMKpi-QjhjhSQhyxgIXaVWpbbcUCd2XNy826WTX1Qu9Wpai6ZS1KWdVlI6oOsVOrut1WSm4krxdmx5d8taw5L1e8WdbFutniqhUbrhTfqLJhqyUOwtiCiKVxW5gYJ9xtt01TLqxo0cb8nsG5wxPkh4xzeu0IuyxGO_WRrZaW_PdaJZlkcXfjyjeup5WX2_5dZL83MBnllk7Gwcs7xANN9GIKdvcTB1FnH_2T8UTGDzPe447_GwAA___eiST4">