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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2978
+            Tmp, ComputeNumSignBits(PN->getIncomingValue(i), Depth + 1, RecQ));
       }
       return Tmp;
----------------
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.


================
Comment at: llvm/unittests/Analysis/ValueTrackingTest.cpp:789
+      EXPECT_EQ(isGuaranteedNotToBePoison(A), true)
+          << "isGuaranteedNotToBePoison does not hold at " << *TI;
+    }
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88360/new/

https://reviews.llvm.org/D88360