Make deadargelim a bit less smart, so it doesn't choke on nested structs as

return values that are still (partially) live. Instead of updating all uses of
a call instruction after removing some elements, it now just rebuilds the
original struct (With undef gaps where the unused values were) and leaves it to
instcombine to clean this up.

The added testcase still fails currently, but this is due to instcombine which
isn't good enough yet. I will fix that part next.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@53608 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 35 insertions, 39 deletions

diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp
index 34e7329..9d29c83 100644
--- a/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -796,45 +796,28 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
       } else {
         assert(STy && "Return type changed, but not into a void. The old "
                       "return type must have been a struct!");
-        // The original return value was a struct, update all uses (which are
-        // all extractvalue instructions, or uses that are unused themselves).
-        for (Value::use_iterator I = Call->use_begin(), E = Call->use_end();
-             I != E;) {
-          if (ExtractValueInst *EV = dyn_cast<ExtractValueInst>(*I)) {
-            // Increment now, since we're about to throw away this use.
-            ++I;
-            assert(EV->hasIndices() && "Return value used by extractvalue without"
-                                       "indices?");
-            unsigned Idx = *EV->idx_begin();
-            if (NewRetIdxs[Idx] != -1) {
-              if (RetTypes.size() > 1) {
-                // We're still returning a struct, create a new extractvalue
-                // instruction with the first index updated
-                std::vector<unsigned> NewIdxs(EV->idx_begin(), EV->idx_end());
-                NewIdxs[0] = NewRetIdxs[Idx];
-                Value *NEV = ExtractValueInst::Create(New, NewIdxs.begin(),
-                                                      NewIdxs.end(), "retval",
-                                                      EV);
-                EV->replaceAllUsesWith(NEV);
-                EV->eraseFromParent();
-              } else {
-                // We are now only returning a simple value, remove the
-                // extractvalue.
-                EV->replaceAllUsesWith(New);
-                EV->eraseFromParent();
-              }
-            } else {
-              // Value unused, replace uses by null for now, they will get removed
-              // later on.
-              EV->replaceAllUsesWith(Constant::getNullValue(EV->getType()));
-              EV->eraseFromParent();
-            }
-          } else {
-            // Not an extractvalue, so this use will become dead soon. Just
-            // replace it with null.
-            I.getUse().set(Constant::getNullValue(I.getUse().get()->getType()));
+        // We used to return a struct. Instead of doing smart stuff with all the
+        // uses of this struct, we will just rebuild it using
+        // extract/insertvalue chaining and let instcombine clean that up.
+        //
+        // Start out building up our return value from undef
+        Value *RetVal = llvm::UndefValue::get(RetTy);
+        for (unsigned i = 0; i != PartialRetVals; ++i)
+          if (NewRetIdxs[i] != -1) {
+            Value *V;
+            if (RetTypes.size() > 1)
+              // We are still returning a struct, so extract the value from our
+              // return value
+              V = ExtractValueInst::Create(New, NewRetIdxs[i], "newret", Call);
+            else
+              // We are now returning a single element, so just insert that
+              V = New;
+            // Insert the value at the old position
+            RetVal = InsertValueInst::Create(RetVal, V, i, "oldret", Call);
           }
-        }
+        // Now, replace all uses of the old call instruction with the return
+        // struct we built
+        Call->replaceAllUsesWith(RetVal);
         New->takeName(Call);
       }
     }
diff --git a/test/Transforms/DeadArgElim/multdeadretval.ll b/test/Transforms/DeadArgElim/multdeadretval.ll
index f60fd0f..6e0474a 100644
--- a/test/Transforms/DeadArgElim/multdeadretval.ll
+++ b/test/Transforms/DeadArgElim/multdeadretval.ll
@@ -38,6 +38,14 @@ define internal {i32, i32, i16} @test5() {
         ret {i32, i32, i16} %C
 }
 
+; Nested return values
+define internal {{i32}, {i16, i16}} @test6() {
+        %A = insertvalue {{i32}, {i16, i16}} undef, i32 1, 0, 0
+        %B = insertvalue {{i32}, {i16, i16}} %A, i16 2, 1, 0
+        %C = insertvalue {{i32}, {i16, i16}} %B, i16 3, 1, 1
+        ret {{i32}, {i16, i16}} %C
+}
+
 define i32 @main() {
         %ret = call {i32, i16} @test2()                ; <i32> [#uses=1]
         %LIVE = extractvalue {i32, i16} %ret, 0
@@ -51,5 +59,10 @@ define i32 @main() {
         %DEAD2 = extractvalue { i32, i32, i16} %ret1, 2
         %V = add i32 %LIVE3, %LIVE4
         %W = add i32 %Z, %V
-        ret i32 %W
+        %ret2 = call { { i32 }, { i16, i16 } } @test6 ()
+        %LIVE5 = extractvalue { { i32 }, { i16, i16 } } %ret2, 0, 0
+        %DEAD3 = extractvalue { { i32 }, { i16, i16 } } %ret2, 1, 0
+        %DEAD4 = extractvalue { { i32 }, { i16, i16 } } %ret2, 1, 1
+        %Q = add i32 %W, %LIVE5
+        ret i32 %Q
 }