From f72d9b1c97b41fff48ad1eecbba59a29c171bff4 Mon Sep 17 00:00:00 2001
From: Johannes Doerfert <jdoerfert@anl.gov>
Date: Fri, 16 Aug 2019 21:59:52 +0000
Subject: [PATCH] [Attributor] Fix: Do not partially resolve returned calls.

By partially resolving returned calls we did not record that they were
not fully resolved which caused odd behavior down the line. We could
also end up with some, but not all, returned values of the callee in the
returned values map of the caller, another odd behavior we want to
avoid.

llvm-svn: 369160
---
 llvm/lib/Transforms/IPO/Attributor.cpp | 39 ++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 882d093..62ec393 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -943,12 +943,35 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
                       << static_cast<const AbstractAttribute &>(RetValAA)
                       << "\n");
 
-    // If we know something but not everyting about the returned values, keep
-    // track of that too. Hence, remember transitively unresolved calls.
-    UnresolvedCalls.insert(RetValAA.getUnresolvedCalls().begin(),
-                           RetValAA.getUnresolvedCalls().end());
+    // Do not try to learn partial information. If the callee has unresolved
+    // return values we will treat the call as unresolved/opaque.
+    auto &RetValAAUnresolvedCalls = RetValAA.getUnresolvedCalls();
+    if (!RetValAAUnresolvedCalls.empty()) {
+      UnresolvedCalls.insert(CB);
+      continue;
+    }
+
+    // Now check if we can track transitively returned values. If possible, thus
+    // if all return value can be represented in the current scope, do so.
+    bool Unresolved = false;
+    for (auto &RetValAAIt : RetValAA.returned_values()) {
+      Value *RetVal = RetValAAIt.first;
+      if (isa<Argument>(RetVal) || isa<CallBase>(RetVal) ||
+          isa<Constant>(RetVal))
+        continue;
+      // Anything that did not fit in the above categories cannot be resolved,
+      // mark the call as unresolved.
+      LLVM_DEBUG(dbgs() << "[AAReturnedValues] transitively returned value "
+                           "cannot be translated: "
+                        << *RetVal << "\n");
+      UnresolvedCalls.insert(CB);
+      Unresolved = true;
+      break;
+    }
+
+    if (Unresolved)
+      continue;
 
-    // Now track transitively returned values.
     for (auto &RetValAAIt : RetValAA.returned_values()) {
       Value *RetVal = RetValAAIt.first;
       if (Argument *Arg = dyn_cast<Argument>(RetVal)) {
@@ -967,12 +990,6 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
         NewRVsMap[RetVal].insert(It.second.begin(), It.second.end());
         continue;
       }
-      // Anything that did not fit in the above categories cannot be resolved,
-      // mark the call as unresolved.
-      LLVM_DEBUG(dbgs() << "[AAReturnedValues] transitively returned value "
-                           "cannot be translated: "
-                        << *RetVal << "\n");
-      UnresolvedCalls.insert(CB);
     }
   }
 
-- 
2.7.4