[PATCH] D90529: Allow nonnull/align attribute to accept poison

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

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

> In D90529#2596245 <https://reviews.llvm.org/D90529#2596245>, @aqjune wrote:
>
>>   call void @llvm.assume(true) ["attr1"(p)]
>>   call void @llvm.assume(true) ["attr2"(p)]
>>     =>
>>   call void @llvm.assume(true) ["attr1"(p), "attr2"(p)] ; synergy between attributes may introduce UB
>>
>> TBH, making things simple and introducing no special rule seems to be the best practice for avoiding possible miscompilations or glitches in spec I think.
>> I see that a similar thing is happening in relaxed concurrency and it is really hard to write at least medium-scale program correctly without relying on commonly-used patterns. :(
>
> I think I missed your "simple" no-special rule suggestion. Could you repeat that or link it?

My suggestion is to keep the LangRef text of assume bundle as it is, and add nonnull/align to a bundle when creating assume only if it was paired with noundef.

  call void @f(nonnull %p)         ; => llvm.assume(true) []
  call void @f(nonnull noundef %p) ; => llvm.assume(true) ["nonnull"(%p)]

Well, it is still doing something (the lowering needs to check the existence of noundef), but I think the principle is already applied to the nonnull/align attributes; they can raise UB only if it is paired with noundef, as LangRef says.
I'm sorry if it sounded a bit aggressive :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90529