[PATCH] D90529: Allow nonnull attribute to accept poison

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

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

> I think we have two choices here:
>
> 1. Don't raise UB when "value attributes" are passed a "wrong value", e.g., `null` for a `nonnull` attribute, but make the value poison. Use `nonull` + `noundef` to make it UB.
> 2. Make all "value attributes" accept poison without raising UB.
>
> If we don't do 1), we should talk about 2) before we make `nonnull` special.

It seems these two choices are entangled.
If `f(nonnull poison)` is okay (in other words, not UB), then `f(nonnull null)` shouldn't be UB as well.
The reason is that `poison` can be folded into `null` in any time.
For example, when `inbounds` is stripped from `v = gep inbounds ...`, `v` can be transformed from `poison` to `null` pointer.

> I fail to see the reason it is different from any other (or at least most) "value attributes".

I had a thought about this, and here's what I think:
Each value attribute has a primary transformation to target.
For example, `nonnull` is for null comparison folding, and `dereferenceable` is for reordering of load instructions by guaranteeing that dereferencing the pointer never raises UB (unless the pointer is freed).

I think `nonnull` is different from `dereferenceable` in that allowing it to be poison doesn't break the intended optimization.
This folding is still okay even if the pointer is poison.
By allowing poison, further optimizations can be done as well because `gep inbounds ptr, 1` can be marked as nonnull.
However, allowing `dereferenceable` to be `poison` will block such optimizations, which doesn't seem quite desirable IMO.

This causes the properties of attributes slightly heterogeneous, but I think it is fine if the goal is to support more optimizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90529