[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:3275
+Additionally, undefined behavior occurs if a side effect, including any
+:ref:`volatile <volatile>` operation, depends on poison.
 
----------------
nlopes wrote:
> This implies that doing a volatile store of a poison to memory is UB. Is this intended? I don't know of a use case that requires such behavior.
One of the examples below explicitly lists volatile store of poison value as UB, which is why I mentioned it here:

```
      store volatile i32 %poison, i32* @g  ; External observation; undefined behavior.
```

The only code I'm aware of that does does reasoning in terms of side-effects with (control) dependence on poison is https://github.com/llvm-mirror/llvm/blob/1cbbb3f527a5aa13eda990e3dfa31f2ca4f64e07/lib/Analysis/ScalarEvolution.cpp#L6010-L6030, which does not care about the classification of any particular operation.

Personally I don't think it makes sense to classify a volatile store of poison as UB for two reasons: First, a volatile operation //can// have side-effects, but does not need to. If it does have side-effects then it is UB, but for the purposes of generic IR reasoning, we have to assume here that it might just be a store to normal memory that happens to be marked volatile. Second, making an operation `volatile` generally allows strictly less transformations to be performed. This would be a case where marking something `volatile` would allow more aggressive optimizations using poison-based reasoning, which does not seem right.

I'd be happy to drop the provision about volatile operations here, as well as the example below.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63044