[PATCH] D90529: Allow nonnull attribute to accept poison

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

In D90529#2369578 <https://reviews.llvm.org/D90529#2369578>, @jdoerfert wrote:

>> In order to regain optimization power, we need a way to attach noundef more aggressively.
>
> This is correct but orthogonal. Having `noundef` "backed-in" to other attributes is not helping us to derive `noundef` it just prevents us to derive other attributes.

I think `dereferenceable`(current UB semantics) != `dereferenceable`(new poison semantics) + `noundef`. In the past I thought they were equal, but here is the reason...

The reason is that the current `dereferenceable` accepts partially undefined pointers, IIUC. At least, LangRef does not prohibit it.
noundef allows accepting well-defined pointers only, so there is a slight difference.

Note that this also happens to `nonnull`: `nonnull` (current UB sem.) != `nonnull`(new poison sem.) + `noundef`.
However, the situation is slightly different, because the change allows more analysis.
`gep inbounds p, 1` is now nonnull, which is certainly a benefit.

In order to regain equivalence, `nopoison` should be introduced. `nopoison` allows the value to be (partially) undef but not poison.
`dereferenceable`(current UB semantics) == `dereferenceable`(new poison semantics) + `nopoison` holds.

This is really due to the complexity of undef.... :(

> Can you clarify "loading poison is UB". I'm unsure I follow/agree.
>
> FWIW, loading a dereferenceable pointer might result in poison but it's even not clear to me that out-of-bounds accesses are UB as of now (in IR).
> Maybe I just forgot where it says I cannot do `load i32, i32* (bitcast i8* [alloca i8] to i32*)`. I certainly would get 3 poison bytes if not UB.

I meant 'loading a poison pointer', sorry. `load i8* poison` raises UB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90529