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

[Florian Hahn via llvm-commits]

Hi,

For some reason this patch requires Administrator permissions to comment on, which I don’t have. I have some suggestions. It would be great if you can change the policy so regular users can comment.

Cheers,
Florian

> On Jul 21, 2020, at 06:00, Congzhe Cao via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> congzhe created this revision.
> congzhe added reviewers: davide, efriedma, mssimpso.
> congzhe created this object with edit policy "Administrators".
> congzhe added projects: fixing bugs in llvm, LLVM.
> Herald added subscribers: llvm-commits, hiraditya.
> 
> In IPSCCP when a function is optimized to return undef, it should clear the returned attribute for all its input arguments.
> 
> The bug is exposed when the value of an input argument of the function is assigned to a physical register and
> because of the argument having a returned attribute, the value of this physical register will continue to be used
> as the function return value right after the call instruction returns, even if the value that this register holds may 
> be clobbered during the function call. This potentially results in incorrect values being used afterwards.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> https://reviews.llvm.org/D84220
> 
> Files:
>  llvm/lib/Transforms/Scalar/SCCP.cpp
>  llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll
> 
> 
> Index: llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll
> ===================================================================
> --- /dev/null
> +++ llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll
> @@ -0,0 +1,22 @@
> +; if IPSCCP determines a function returns undef,
> +; then the "returned" attribute of input arguments
> +; should be cleared.
> +
> +; RUN: opt < %s -ipsccp -S | FileCheck %s
> +define i32 @main() {
> +; CHECK-LABEL: @main
> +entry:
> +; CHECK-NEXT: entry:
> +  %call = call i32 @func_return_undef(i32 1)
> +; CHECK: {{call.*@func_return_undef}}
> +  ret i32 0
> +}
> +
> +define internal i32 @func_return_undef(i32 returned %arg) {
> +; CHECK: {{define.*@func_return_undef}}
> +; CHECK-NOT: returned
> +entry:
> +; CHECK-NEXT: entry:
> +; CHECK-NEXT: {{ret.*undef}}
> +ret i32 %arg
> +}
> Index: llvm/lib/Transforms/Scalar/SCCP.cpp
> ===================================================================
> --- llvm/lib/Transforms/Scalar/SCCP.cpp
> +++ llvm/lib/Transforms/Scalar/SCCP.cpp
> @@ -2038,6 +2038,8 @@
>   for (unsigned i = 0, e = ReturnsToZap.size(); i != e; ++i) {
>     Function *F = ReturnsToZap[i]->getParent()->getParent();
>     ReturnsToZap[i]->setOperand(0, UndefValue::get(F->getReturnType()));
> +    for (Argument& A : F->args())
> +      F->removeParamAttr(A.getArgNo(), Attribute::Returned);
>   }
> 
>   // If we inferred constant or undef values for globals variables, we can
> 
> 
> <D84220.279422.patch>