[PATCH] D63044: [LangRef] Clarify poison semantics

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added inline comments.


================
Comment at: llvm/docs/LangRef.rst:3274
+-  The divisor operand of a ``udiv``, ``sdiv``, ``urem`` or ``srem``
+   instruction.
+
----------------
aqjune wrote:
> How about mentioning right-shift operations on poison (e.g. `ashr x, poison`) as well?
> Its semantics is equivalent to `x / 2^y` when x and y are non-poison(and non-undef), but LLVM seems to hoist `ashr x, poison` during optimization, implying that it cannot be UB : https://godbolt.org/z/2LG0xX .
`ashr` does not have any special semantics in this context: It propagates poison, i.e. `ashr x, poison` is poison again. (Oversize shifts are defined as poison, not as UB.)


================
Comment at: llvm/docs/LangRef.rst:3276
+
+Additionally, undefined behavior occurs if a side effect depends on poison.
 
----------------
aqjune wrote:
> jdoerfert wrote:
> > Now, side effects, what do we count and why do we have it explicitly.
> > 
> > Also, should we explicitly mention control flow or is it sufficiently covered by the "*depends*" definition above?
> I also think that control flow on poison is an important issue.
> 
> https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Scalar/LoopUnswitch.cpp#L585 also mentions that there's discrepancy between semantics of branch on undef/poison.
> My suggestion is to mention the discrepancy explicitly here so people can acknowledge the issue while reading LangRef.
> Now, side effects, what do we count and why do we have it explicitly.

I don't think there is anything in the LLVM IR where we can say that it definitely has a side-effect, just operations that might have one (volatile, calls, etc). We need this to be UB to allow reasoning like https://github.com/llvm-mirror/llvm/blob/1cbbb3f527a5aa13eda990e3dfa31f2ca4f64e07/lib/Analysis/ScalarEvolution.cpp#L6010-L6030, but don't really care about what exactly a side-effect is -- just that if one exists, a dependence on poison is UB.

> Also, should we explicitly mention control flow or is it sufficiently covered by the "*depends*" definition above?

Control flow is covered by the dependence definition, but as this is an important case, I've added an explicit note now.


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

https://reviews.llvm.org/D63044