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

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

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

> In D90529#2594677 <https://reviews.llvm.org/D90529#2594677>, @aqjune wrote:
>
>> In D90529#2594464 <https://reviews.llvm.org/D90529#2594464>, @jdoerfert wrote:
>>
>>> In D90529#2594026 <https://reviews.llvm.org/D90529#2594026>, @aqjune wrote:
>>>
>>>> While writing a LangRef patch for the semantic changes in nonnull & align attributes in assume operand bundle, I found one interesting case. :)
>>>>
>>>> Creating assume from these two calls yields the same result:
>>>>
>>>>   call void @hi1(i8* nonnull %val, i8* noundef %val)
>>>>   call void @hi2(i8* nonnull noundef %val)
>>>>
>>>> They both generate:
>>>>
>>>>   call void @llvm.assume(true)["nonnull"(i8* %val), "noundef"(i8* %val)]
>>>>
>>>> A question is whether this assume is UB if, say, %val is `inttoptr(1)`.
>>>> Creating this assume from `call void @hi2(%val)` is okay because `call void @hi2(%val)` is already UB.
>>>> But, creating the assume from `call void @hi1(%val, %val)` isn't okay! `%val` is non-null and noundef, so the call is fine.
>>>>
>>>> To distinguish these two, what about this?
>>>> (1) simply use UB semantics for nonnull attribute in assume operand bundle
>>>> (2) lower `nonnull` attribute in the call-site into `nonnull` bundle only if it is accompanied with `noundef`.
>>>> Then, we don't need to change anything about nonnull from LangRef: the current text is defining it as UB (https://llvm.org/docs/LangRef.html#assume-operand-bundles ).
>>>
>>> I thought at first (2) is the way to go but then I asked myself if `@hi1(%val, %val)` is really not already UB. The question is, does `val` become poison for this use or is `val` poison in the scope of the use. I feel the latter is what we actually want. If we assume this is the only call of `hi1` and we then do interprocedural transformations to replace the use of the first argument with the second or vice versa, would that be illegal because of the attribute mismatch or allowed? If it is the latter, the call must have been UB.
>>
>> Hmm, I think the issue happens with a call without noundef as well:
>>
>>   call void @hi1(nonnull %val, %val) ; let's assume that this is the only call
>>   
>>   define void @hi(i8* %x, i8* %y) {
>>     use(%x, %y) ; this cannot be use(%x, %x)!
>>   }
>>
>> To do this replacement, nonnull at the call site should be dropped first.
>>
>> Besides this, I think expanding the scope of being poison to the same variables makes optimizations like CSE hard. For example:
>>
>>   call void @hi1(<attr> %val, <attr2> %val2)
>>
>> If CSE concludes that `%val` is `%val2`, it can remove `%val2` and optimize it into:
>>
>>   call void @hi1(<attr> %val, <attr2> %val)
>>
>> .. and this can unexpectedly introduce UB due to the synergy between <attr> and <attr2>. I guess fixing optimizations to consider attributes might be a bit costly.. :/
>
> That is/was already the case, right? So, is the current behavior OK or not, potentially in light of making `nonnull` produce poison not UB.
> I think it is/was OK, and, TBH I think it has to be. If we look at it scoped based it all makes sense (I think/hope).
> On a practical note, you might not see all the call sites so "dropping first" doesn't work if you know that x == y from a local assume (in the hi example above).

I don't understand this sentence, I thought we could see all the call sites and assume that it was the only call of `hi1`.

  If we assume this is the only call of `hi1` and we then do interprocedural transformations to replace the use of the first argument with the second or vice versa, would that be illegal because of the attribute mismatch or allowed?



> If the attribute holds for the value in the scope, CSE is free to change val to val2 because the attributes did already conflict even if you used val and val2.
> Do you see a problem when the attribute is scope based?

Before diving into further discussion, let's clarify my understanding about the semantics that you want:

- Calling `f(nonnull null, noundef null)` is equivalent to `f(nonnull noundef null, nonnull noundef null)` because an attribute applies to all 'equivalent' values in the arguments. Is my understanding correct?
- Similarly, calling `f(nonnull null, null)` is equivalent to `f(poison, poison)`?
- What about `f(nonnull poison, null)`? Since poison can be folded into any value, we can make it `f(nonnull null, null)`, which is again `f(poison, poison)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90529