[PATCH] D64215: Add a transform pass to make the executable semantics of poison explicit in the IR

[Philip Reames via Phabricator via llvm-commits]

reames marked 6 inline comments as done.
reames added inline comments.


================
Comment at: lib/Transforms/Instrumentation/PoisonChecking.cpp:91
+
+static Value *buildOrChain(IRBuilder<> &B, ArrayRef<Value*> Ops) {
+  if (Ops.size() == 0)
----------------
sanjoy wrote:
> This looks like it can live on `IRBuilder`.
Agreed, plan to move it there once I reapply the split out fix which got reverted due to a couple of tests I didn't catch.


================
Comment at: lib/Transforms/Instrumentation/PoisonChecking.cpp:95
+  unsigned i = 0;
+  for (; i < Ops.size() && isConstantFalse(Ops[i]); i++) {}
+  if (i == Ops.size())
----------------
sanjoy wrote:
> Is all of this necessary?  I'd expect one round of instsimplify to fix up this kind of stuff.
It makes the test much, much easier to read.  :)


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

https://reviews.llvm.org/D64215