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

diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp
index 34e73295d770f71d1af013906e7475971936e4c7..9d29c8366e29057d52707e5c7d51f0baddf9d07c 100644 (file)
--- 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 f60fd0fc395333d37d0e08552248a213bbfe72a4..6e0474a6f8d25eadf6a008ab5d8f368c8846594a 100644 (file)
--- 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
 }