[PATCH] D90529: Allow nonnull attribute to accept poison
Juneyoung Lee via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 2 13:02:08 PST 2020
aqjune added a comment.
In D90529#2368303 <https://reviews.llvm.org/D90529#2368303>, @jdoerfert wrote:
> Fair. Though, I think we want to produce poison for one set of attributes for which the name "value attribute" was not well chosen.
> So far, the things I think should produce poison not UB are:
>
> (pure) value attributes:
>
> - nonnull
> - align
> - [used_bits] (not existing yet)
>
> (context) value attributes:
>
> - dereferenceable
> - dereferenceable_or_null
> - [object_size] (as proposed on the list)
>
> WDYT?
In case of `dereferenceable`, my opinion is still slightly different: it represents the property of the memory at the point.
If a memory block is freed, a same pointer value won't be dereferenceable after the deallocation.
Other than the conceptual reason, my practical concern about `dereferenceable` is that `dereferenceable` is hard to use unless it is with `noundef`.
Since loading poison is UB, the attribute can't still give a guarantee that loading the pointer is well defined.
Furthermore, `noundef` is hard to infer from the context. For example,
store i32 0, i32* %p
call void @f(i32* %p) ; can we infer %p's noundef & dereferenceable(4) from store?
`store i32 0, i32* %p` does not guarantee that `%p` is noundef because it can be partially undef but still dereferenceable (D87994 <https://reviews.llvm.org/D87994> is a relevant LangRef patch).
The example above was propagating the attribute from caller to callee, but the inverse direction (callee -> caller) is the same: If `@f` contains a store, it guarantees that the pointer is dereferenceable but not noundef.
In order to regain optimization power, we need a way to attach noundef more aggressively.
D81678 <https://reviews.llvm.org/D81678> will attach noundef to arguments, and additionally I think it is valid for clang to attach `!noundef` when lowering non-char typed lvalues to load.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90529/new/
https://reviews.llvm.org/D90529
More information about the llvm-commits
mailing list