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

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

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?
>
>> The only reason I can think of we wouldn't want the instant UB is if it interacts badly with some significant number of optimizations.  I'm not coming up with anything it could interact badly with off the top of my head, though, and any transform that did have an issue would also interact badly with noundef.
>
> I agree that it would interact badly with `noundef` but I was always thinking that would be less of an issue, maybe I'm wrong though.
>
> When it comes to optimizations, I think this shows how problematic the UB semantic is. The codegen interpretation is the problem not IPSCCP (IMHO). Even if you disagree, I doubt this will fix it. Without running it I would suspect https://godbolt.org/z/rG48nz to have at least one `returned` argument in a `ret undef` function. @congzhe could you try this?

Thanks for the comment, with this patch the output after "opt -ipsccp" is

  define i32 @main() {
    %call = call i32 @func_return_undef_outer(i32 1)
    ret i32 0
  }
  
  define internal i32 @func_return_undef_inner(i32 %arg) {
    ret i32 undef
  }
  
  define internal i32 @func_return_undef_outer(i32 %arg) {
    %call = call i32 @func_return_undef_inner(i32 1)
    ret i32 undef
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84220