[llvm] [IR][JumpThreading] Fix infinite recursion on compare self-reference [updated] (PR #129501)

[Robert Imschweiler via llvm-commits]

================
@@ -1519,12 +1526,24 @@ Constant *JumpThreadingPass::evaluateOnPredecessorEdge(BasicBlock *BB,
   }
 
   // If we have a CmpInst, try to fold it for each incoming edge into PredBB.
+  // Note that during the execution of the pass, phi nodes may become constant
+  // and may be removed, which can lead to self-referencing instructions in
+  // code that becomes unreachable. Consequently, we need to handle those
+  // instructions in unreachable code and check before going into recursion.
   if (CmpInst *CondCmp = dyn_cast<CmpInst>(V)) {
     if (CondCmp->getParent() == BB) {
-      Constant *Op0 =
-          evaluateOnPredecessorEdge(BB, PredPredBB, CondCmp->getOperand(0), DL);
-      Constant *Op1 =
-          evaluateOnPredecessorEdge(BB, PredPredBB, CondCmp->getOperand(1), DL);
+      Constant *Op0 = nullptr;
+      Constant *Op1 = nullptr;
+      if (Value *V0 = CondCmp->getOperand(0); !Visited.contains(V0)) {
+        Visited.insert(V0);
----------------
ro-i wrote:

> I think it would be cleaner to do these checks in evaluateOnPredecessorEdge(), not at each call to evaluateOnPredecessorEdge. Doesn't really matter right now, but will make sure this keeps getting handled if this code handles more than icmp in the future.

I adapted the code to insert and double check at the start of evaluateOnPredecessorEdge, see my latest commit. However, there still needs to be a `contains` before each call because, otherwise, the caller doesn't know whether it's allowed to erase the Value after the call.

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