[llvm] [InstCombine] Replace an integer comparison of a `phi` node with multiple `ucmp`/`scmp` operands and a constant with `phi` of individual comparisons of original intrinsic's arguments (PR #107769)

[Nikita Popov via llvm-commits]

================
@@ -1844,11 +1856,26 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) {
   // inserting the computation on some other paths (e.g. inside a loop).  Only
   // do this if the pred block is unconditionally branching into the phi block.
   // Also, make sure that the pred block is not dead code.
-  if (NonSimplifiedBB != nullptr) {
-    BranchInst *BI = dyn_cast<BranchInst>(NonSimplifiedBB->getTerminator());
-    if (!BI || !BI->isUnconditional() ||
-        !DT.isReachableFromEntry(NonSimplifiedBB))
+  // After checking for all of the above, clone the instruction that uses the
+  // phi node and move it into the incoming BB because we know that the next
+  // iteration of InstCombine will simplify it.
+  for (auto OpIndex : OpsToMoveUseTo) {
+    Value *Op = PN->getIncomingValue(OpIndex);
+    BasicBlock *OpBB = PN->getIncomingBlock(OpIndex);
+
+    BranchInst *BI = dyn_cast<BranchInst>(OpBB->getTerminator());
+    if (!BI || !BI->isUnconditional() || !DT.isReachableFromEntry(OpBB))
       return nullptr;
+
+    Instruction *Clone = I.clone();
+    for (Use &U : Clone->operands()) {
+      if (U == PN)
+        U = Op;
+      else
+        U = U->DoPHITranslation(PN->getParent(), OpBB);
+    }
+    Clone = InsertNewInstBefore(Clone, OpBB->getTerminator()->getIterator());
+    NewPhiValues[OpIndex] = Clone;
----------------
nikic wrote:

Combining this in one loop is problematic, because you may end up first cloning some values and then later aborting the transform. This can likely lead to an infinite combine loop.

I think the check of BranchInst above should really be part of the previous loop already, around the place where InvokeInst is checked.

https://github.com/llvm/llvm-project/pull/107769