[llvm-commits] [llvm] r53608 - in /llvm/trunk: lib/Transforms/IPO/DeadArgumentElimination.cpp test/Transforms/DeadArgElim/multdeadretval.ll

[Matthijs Kooijman]

Author: matthijs
Date: Tue Jul 15 09:03:10 2008
New Revision: 53608

URL: http://llvm.org/viewvc/llvm-project?rev=53608&view=rev
Log:
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.

Modified:
    llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp
    llvm/trunk/test/Transforms/DeadArgElim/multdeadretval.ll

Modified: llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=53608&r1=53607&r2=53608&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp Tue Jul 15 09:03:10 2008
@@ -796,45 +796,28 @@
       } 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);
       }
     }

Modified: llvm/trunk/test/Transforms/DeadArgElim/multdeadretval.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadArgElim/multdeadretval.ll?rev=53608&r1=53607&r2=53608&view=diff

==============================================================================
--- llvm/trunk/test/Transforms/DeadArgElim/multdeadretval.ll (original)
+++ llvm/trunk/test/Transforms/DeadArgElim/multdeadretval.ll Tue Jul 15 09:03:10 2008
@@ -38,6 +38,14 @@
         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 @@
         %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
 }