[PATCH] D78152: [ValueTracking] Implement impliesPoison

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM with some nits. Please also fix the clang-tidy warnings wrt const.

There's various ways this could be improved, but I guess we should start simple and see which useful cases slip by.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4806
+  // that ValAssumedPoison is poison.
+  SmallSet<const Value *, 4> PoisonValues;
+  PoisonValues.insert(ValAssumedPoison);
----------------
SmallPtrSet


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4845
+
+  if (PoisonValues.count(V) != 0)
+    return true;
----------------
count -> contains


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4867
   return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true);
 }
 
----------------
You probably want to leave these two functions next to the main `canCreateUndefOrPoison()` definition.


================
Comment at: llvm/unittests/Analysis/ValueTrackingTest.cpp:703
+                "}");
+  EXPECT_EQ(impliesPoison(A, A), 1u);
+}
----------------
Why `1u` and not `true`? Or for that matter, `EXPECT_TRUE()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78152