From 7f75841a739bb92d2913b7529061eb395a33bb64 Mon Sep 17 00:00:00 2001
From: Tim Northover <tnorthover@apple.com>
Date: Wed, 11 Feb 2015 23:13:11 +0000
Subject: [PATCH] DeadArgElim: aggregate Return assessment properly.

I mistakenly thought the liveness of each "RetVal(F, i)" depended only on F. It
actually depends on the index too, which means we need to be careful about how
the results are combined before return. In particular if a single Use returns
Live, that counts for the entire object, at the granularity we're considering.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@228885 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../IPO/DeadArgumentElimination.cpp           | 11 ++++---
 test/Transforms/DeadArgElim/aggregates.ll     | 30 +++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp
index 596204dac62..44313113fce 100644
--- a/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -449,12 +449,15 @@ DAE::Liveness DAE::SurveyUse(const Use *U,
         // We might be live, depending on the liveness of Use.
         return MarkIfNotLive(Use, MaybeLiveUses);
       } else {
-        DAE::Liveness Result;
+        DAE::Liveness Result = MaybeLive;
         for (unsigned i = 0; i < NumRetVals(F); ++i) {
           RetOrArg Use = CreateRet(F, i);
-          // We might be live, depending on the liveness of Use. All Results
-          // should be the same since they depend only on F.
-          Result = MarkIfNotLive(Use, MaybeLiveUses);
+          // We might be live, depending on the liveness of Use. If any
+          // sub-value is live, then the entire value is considered live. This
+          // is a conservative choice, and better tracking is possible.
+          DAE::Liveness SubResult = MarkIfNotLive(Use, MaybeLiveUses);
+          if (Result != Live)
+            Result = SubResult;
         }
         return Result;
       }
diff --git a/test/Transforms/DeadArgElim/aggregates.ll b/test/Transforms/DeadArgElim/aggregates.ll
index 84370834e11..f54c6c9ea44 100644
--- a/test/Transforms/DeadArgElim/aggregates.ll
+++ b/test/Transforms/DeadArgElim/aggregates.ll
@@ -130,3 +130,33 @@ define i32 @test_ret_applies_to_all() {
   %ret = extractvalue {i32, i32} %val, 1
   ret i32 %ret
 }
+
+; Case 6: When considering @mid, the return instruciton has sub-value 0
+; unconditionally live, but 1 only conditionally live. Since at that level we're
+; applying the results to the whole of %res, this means %res is live and cannot
+; be reduced. There is scope for further optimisation here (though not visible
+; in this test-case).
+
+; CHECK-LABEL: define internal { i8*, i32 } @inner()
+
+define internal {i8*, i32} @mid() {
+  %res = call {i8*, i32} @inner()
+  %intval = extractvalue {i8*, i32} %res, 1
+  %tst = icmp eq i32 %intval, 42
+  br i1 %tst, label %true, label %true
+
+true:
+  ret {i8*, i32} %res
+}
+
+define internal {i8*, i32} @inner() {
+  ret {i8*, i32} {i8* null, i32 42}
+}
+
+define internal i8 @outer() {
+  %res = call {i8*, i32} @mid()
+  %resptr = extractvalue {i8*, i32} %res, 0
+
+  %val = load i8* %resptr
+  ret i8 %val
+}
\ No newline at end of file
-- 
2.34.1