[PATCH] D84220: [IPSCCP] Fix a bug that the "returned" attribute is not cleared when function is optimized to return undef

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D84220#2182687 <https://reviews.llvm.org/D84220#2182687>, @fhahn wrote:

> In D84220#2180375 <https://reviews.llvm.org/D84220#2180375>, @jdoerfert wrote:
>
>> In D84220#2180340 <https://reviews.llvm.org/D84220#2180340>, @efriedma wrote:
>>
>>> The current definition of "returned" in LangRef is unclear, yes.
>>>
>>> In terms of what we actually want, the "must be equal or else instant UB" variation seems more useful than the "must be equal or poison" variation.  With "must be equal or poison", basically the only legal optimization is replacing the result with the argument.
>>
>> I always thought that was the purpose. Could you elaborate what else we want to do?
>
> Would poison limit the way we can use other information for the returned value, e.g. range information?
>
> For example, consider the (arguably a bit contrived) code below. We know that `%limit` is [0, 2), not poison, and that it is UB for `@foo` to return anything other than `%limit`. It should be legal to replace `%res` with `%r.f`. https://alive2.llvm.org/ce/z/NrdV6x
>
> But can we do the same transform if `@foo` returns poison instead of immediate UB on an invalid return value? In case it returned poison, `%r.f` would be any `i4`, while in the original function it would be in the range [0, 2).
>
>   declare i4 @foo(i4 returned %arg)
>   
>   define i4 @src(i4 %x) {
>     %x.f = freeze i4 %x
>     %limit = and i4 %x.f, 1
>     %r = call i4 @foo(i4 %limit)
>     %r.f = freeze i4 %r
>     %res = and i4 %r.f, 1
>     ret i4 %res
>   }

Right. And if you wanted to say, this has to be returned or it is UB, you simply add `noundef` as an attribute to the return value. The poison proposal doesn't limit us in any way due to `noundef`.

> Thanks for the comment, for the IR provided in https://godbolt.org/z/rG48nz, with this patch the output after "opt -ipsccp" is

Yes, thanks for checking.

I guess if we fix up every pass that modifies returned instructions we can do this. The Attributor is probably broken too.
That said, I still believe there is is bandaging the underlying problem. I don't have a really good example handy right now though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84220