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

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added inline comments.


================
Comment at: lib/Transforms/Instrumentation/PoisonChecking.cpp:105
+    return buildOrChain(B, Checks);
+  }
+
----------------
This is great.
Is it assumed that poison is always value-wise?
In other words, if the result is 1, does it mean either:
(1) the value is always fully poison
(2) the value has at least one poison 'bit'?

In the future, there might exist several kinds of poison semantics that people would like to test (e.g. bitwise poison vs. valuewise poison) - so I think it is a good idea to describe which poison semantics this visitAdd is assuming.


================
Comment at: lib/Transforms/Instrumentation/PoisonChecking.cpp:124
+    return buildOrChain(B, Checks);
+  }
+};
----------------
How about merging the contents of `visitSub` and `visitAdd` as they are equivalent except the intrinsics name? It can be reused for `visitMul` as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64215