[llvm] ec489ae - [IPSCCP] Fix a bug that the "returned" attribute is not cleared when function is optimized to return undef

[Danilo C. Grael via llvm-commits]

Author: Congzhe Cao
Date: 2020-09-02T11:21:48-04:00
New Revision: ec489ae048fd971b22400c61458a5295eeba368a

URL: https://github.com/llvm/llvm-project/commit/ec489ae048fd971b22400c61458a5295eeba368a
DIFF: https://github.com/llvm/llvm-project/commit/ec489ae048fd971b22400c61458a5295eeba368a.diff

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

In IPSCCP when a function is optimized to return undef, it should clear the returned attribute for all its input arguments
and its corresponding call sites.

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.

Reviewed By: jdoerfert, fhahn

Differential Revision: https://reviews.llvm.org/D84220

Added: 
    llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll

Modified: 
    llvm/lib/Transforms/Scalar/SCCP.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
index 57befc9c3cfb..2afc778ed821 100644
--- a/llvm/lib/Transforms/Scalar/SCCP.cpp
+++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -23,6 +23,7 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
@@ -2112,9 +2113,27 @@ bool llvm::runIPSCCP(
   }
 
   // Zap all returns which we've identified as zap to change.
+  SmallSetVector<Function *, 8> FuncZappedReturn;
   for (unsigned i = 0, e = ReturnsToZap.size(); i != e; ++i) {
     Function *F = ReturnsToZap[i]->getParent()->getParent();
     ReturnsToZap[i]->setOperand(0, UndefValue::get(F->getReturnType()));
+    // Record all functions that are zapped.
+    FuncZappedReturn.insert(F);
+  }
+
+  // Remove the returned attribute for zapped functions and the
+  // corresponding call sites.
+  for (Function *F : FuncZappedReturn) {
+    for (Argument &A : F->args())
+      F->removeParamAttr(A.getArgNo(), Attribute::Returned);
+    for (Use &U : F->uses()) {
+      // Skip over blockaddr users.
+      if (isa<BlockAddress>(U.getUser()))
+        continue;
+      CallBase *CB = cast<CallBase>(U.getUser());
+      for (Use &Arg : CB->args())
+        CB->removeParamAttr(CB->getArgOperandNo(&Arg), Attribute::Returned);
+    }
   }
 
   // If we inferred constant or undef values for globals variables, we can

diff  --git a/llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll b/llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll
new file mode 100644
index 000000000000..d8b5fbff4e62
--- /dev/null
+++ b/llvm/test/Transforms/SCCP/ipsccp-clear-returned.ll
@@ -0,0 +1,62 @@
+; 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 returned 1)
+; CHECK: call i32 @func_return_undef(i32 1)
+; CHECK-NOT: returned
+  ret i32 %call
+; CHECK: ret i32 1
+}
+
+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
+}
+
+
+; The only case that users of zapped functions are non-call site
+; users is that they are blockaddr users. Skip them because we
+; want to remove the returned attribute for call sites
+
+; CHECK: {{define.*@blockaddr_user}}
+; CHECK-NOT: returned
+define internal i32 @blockaddr_user(i1 %c, i32 returned %d) {
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  br label %branch.block
+
+bb2:
+  br label %branch.block
+
+branch.block:
+  %addr = phi i8* [blockaddress(@blockaddr_user, %target1), %bb1], [blockaddress(@blockaddr_user, %target2), %bb2]
+  indirectbr i8* %addr, [label %target1, label %target2]
+
+target1:
+  br label %target2
+
+; CHECK: ret i32 undef
+target2:
+  ret i32 %d
+}
+
+define i32 @call_blockaddr_user(i1 %c) {
+; CHECK-LABEL: define i32 @call_blockaddr_user(
+; CHECK-NEXT: %r = call i32 @blockaddr_user(i1 %c
+; CHECK-NOT: returned
+; CHECK-NEXT: ret i32 10
+  %r = call i32 @blockaddr_user(i1 %c, i32 returned 10)
+  ret i32 %r
+}