[PATCH] D90529: Allow nonnull/align attribute to accept poison
Juneyoung Lee via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 1 19:52:37 PST 2021
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
More information about the llvm-commits
mailing list