[PATCH] D19212: Have isKnownNotFullPoison be smarter around control flow

[Bjarke Hammersholt Roune via llvm-commits]

broune accepted this revision.
This revision is now accepted and ready to land.

================
Comment at: lib/Analysis/ValueTracking.cpp:3413
@@ -3412,3 +3412,3 @@
 
     case Instruction::Br:
       if (cast<BranchInst>(I)->isConditional())
----------------
As far as I can tell, this code for Br is not in the official repository, yet it is not listed as a changed line in the review. 

https://github.com/llvm-mirror/llvm/blob/615a6cf40aa9c7b280fd665e4ad66d4e0d5395fe/lib/Analysis/ValueTracking.cpp#L3522

Maybe I'm just confused about how diffs are shown or this was added very recently?

================
Comment at: lib/Analysis/ValueTracking.cpp:3442
@@ -3446,7 +3441,3 @@
 
-    // Mark poison that propagates from I through uses of I.
-    if (YieldsPoison.count(&*I)) {
-      for (const User *User : I->users()) {
-        const Instruction *UserI = cast<Instruction>(User);
-        if (UserI->getParent() == BB && propagatesFullPoison(UserI))
-          YieldsPoison.insert(User);
+  while (true) {
+    for (auto &I : make_range(Begin, End)) {
----------------
Would it make sense to have a limit on how many instructions/basic blocks this will consider?

================
Comment at: lib/Analysis/ValueTracking.cpp:3462
@@ +3461,3 @@
+
+    if (auto *NextBB = BB->getSingleSuccessor())
+      if (Visited.insert(NextBB).second) {
----------------
Could there be braces for this multi-line if?


http://reviews.llvm.org/D19212