[ValueTracking] Fix analyses to update CxtI to be phi's incoming edges' terminators

[aqjune]

Summary

It was mentioned that D88276 that when a phi node is visited, terminators at their incoming edges should be used for CtxI.
This is a patch that makes two functions (ComputeNumSignBitsImpl, isGuaranteedNotToBeUndefOrPoison) to do so.

[nikic]

llvm/lib/Analysis/ValueTracking.cpp

2977

The separation of the first incoming value here looks unnecessary, write it like this instead?

Tmp = TyBits;
for (unsigned i = 0, e = NumIncomingValues; i != e; ++i) {
  if (Tmp == 1) return Tmp;
  RecQ.CxtI = PN->getIncomingBlock(i)->getTerminator();
  Tmp = std::min(
      Tmp, ComputeNumSignBits(PN->getIncomingValue(i), Depth + 1, RecQ));
}

Not really related to your change, but it does make it more visible.

llvm/unittests/Analysis/ValueTrackingTest.cpp

789

This test looks like it would be passing both before and after your change. You're also not passing TI to the function, so the failure message doesn't really make sense.

[aqjune]

Address comments

[aqjune]

llvm/unittests/Analysis/ValueTrackingTest.cpp

789

Sorry, my mistake. Fixed

[jdoerfert]

Nit: consider Q.getWithInstruction, no strong feelings.

[nikic]

LGTM

llvm/lib/Analysis/ValueTracking.cpp

case Instruction::PHI: {
       unsigned NumIncomingValues = PN->getNumIncomingValues();
       // Don't analyze large in-degree PHIs.
       if (NumIncomingValues > 4) break;
       // Unreachable blocks may have zero-operand PHI nodes.
       if (NumIncomingValues == 0) break;
 
       // Take the minimum of all incoming values.  This can't infinitely loop
       // because of our depth threshold.
-      Tmp = ComputeNumSignBits(PN->getIncomingValue(0), Depth + 1, Q);
-      for (unsigned i = 1, e = NumIncomingValues; i != e; ++i) {
+      Query RecQ = Q;
+      Tmp = TyBits;
+      for (unsigned i = 0, e = NumIncomingValues; i != e; ++i) {
         if (Tmp == 1) return Tmp;
+        RecQ.CxtI = PN->getIncomingBlock(i)->getTerminator();
         Tmp = std::min(
-            Tmp, ComputeNumSignBits(PN->getIncomingValue(i), Depth + 1, Q));
+            Tmp, ComputeNumSignBits(PN->getIncomingValue(i), Depth + 1, RecQ));
       }
       return Tmp;
     }
 
     case Instruction::Trunc:
       // FIXME: it's tricky to do anything useful for this, but it is an
       // important case for targets like X86.
       break;
 
if (auto *Opr = dyn_cast<Operator>(V)) {
     if (isa<FreezeInst>(V))
       return true;
 
     if (const auto *CB = dyn_cast<CallBase>(V)) {
       if (CB->hasRetAttr(Attribute::NoUndef))
         return true;
     }
 
-    if (!canCreateUndefOrPoison(Opr) && all_of(Opr->operands(), OpCheck))
+    if (const auto *PN = dyn_cast<PHINode>(V)) {
+      unsigned Num = PN->getNumIncomingValues();
+      bool IsWellDefined = true;
+      for (unsigned i = 0; i < Num; ++i) {
+        auto *TI = PN->getIncomingBlock(i)->getTerminator();
+        if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), TI, DT,
+                                              Depth + 1, PoisonOnly)) {
+          IsWellDefined = false;
+          break;
+        }
+      }
+      if (IsWellDefined)
+        return true;
+    } else if (!canCreateUndefOrPoison(Opr) && all_of(Opr->operands(), OpCheck))
       return true;
   }
 
   if (programUndefinedIfUndefOrPoison(V, PoisonOnly))
     return true;
 
   // CxtI may be null or a cloned instruction.
   if (!CtxI || !CtxI->getParent() || !DT)

llvm/unittests/Analysis/ValueTrackingTest.cpp

for (auto &BB : *F) {
     if (&BB == &F->getEntryBlock())
       continue;
 
     EXPECT_EQ(isGuaranteedNotToBePoison(A, BB.getTerminator(), &DT), true)
         << "isGuaranteedNotToBePoison does not hold at " << *BB.getTerminator();
   }
 }
 
+TEST_F(ValueTrackingTest, isGuaranteedNotToBePoison_phi) {
+  parseAssembly("declare i32 @any_i32(i32)"
+                "define void @test() {\n"
+                "ENTRY:\n"
+                "  br label %LOOP\n"
+                "LOOP:\n"
+                "  %A = phi i32 [0, %ENTRY], [%A.next, %NEXT]\n"
+                "  %A.next = call i32 @any_i32(i32 %A)\n"
+                "  %cond = icmp eq i32 %A.next, 0\n"
+                "  br i1 %cond, label %NEXT, label %EXIT\n"
+                "NEXT:\n"
+                "  br label %LOOP\n"
+                "EXIT:\n"
+                "  ret void\n"
+                "}\n");
+  DominatorTree DT(*F);
+  for (auto &BB : *F) {
+    if (BB.getName() == "LOOP") {
+      EXPECT_EQ(isGuaranteedNotToBePoison(A, A, &DT), true)
+          << "isGuaranteedNotToBePoison does not hold";
+    }
+  }
+}
+
 TEST_F(ValueTrackingTest, isGuaranteedNotToBeUndefOrPoison) {
   parseAssembly("declare void @f(i32 noundef)"
                 "define void @test(i32 %x) {\n"
                 "  %A = bitcast i32 %x to i32\n"
                 "  call void @f(i32 noundef %x)\n"
                 "  ret void\n"
                 "}\n");
   EXPECT_EQ(isGuaranteedNotToBeUndefOrPoison(A), true);