[llvm] r228559 - DeadArgElim: fix mismatch in accounting of array return types.
Tim Northover
tnorthover at apple.com
Sun Feb 8 17:21:01 PST 2015
Author: tnorthover
Date: Sun Feb 8 19:21:00 2015
New Revision: 228559
URL: http://llvm.org/viewvc/llvm-project?rev=228559&view=rev
Log:
DeadArgElim: fix mismatch in accounting of array return types.
Some parts of DeadArgElim were only considering the individual fields
of StructTypes separately, but others (where insertvalue &
extractvalue instructions occur) also looked into ArrayTypes.
This one is an actual bug; the mismatch can lead to an argument being
considered used by a return sub-value that isn't being tracked (and
hence is dead by default). It then gets incorrectly eliminated.
Modified:
llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp
llvm/trunk/test/Transforms/DeadArgElim/aggregates.ll
Modified: llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=228559&r1=228558&r2=228559&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp Sun Feb 8 19:21:00 2015
@@ -387,14 +387,32 @@ bool DAE::RemoveDeadArgumentsFromCallers
/// for void functions and 1 for functions not returning a struct. It returns
/// the number of struct elements for functions returning a struct.
static unsigned NumRetVals(const Function *F) {
- if (F->getReturnType()->isVoidTy())
+ Type *RetTy = F->getReturnType();
+ if (RetTy->isVoidTy())
return 0;
- else if (StructType *STy = dyn_cast<StructType>(F->getReturnType()))
+ else if (StructType *STy = dyn_cast<StructType>(RetTy))
return STy->getNumElements();
+ else if (ArrayType *ATy = dyn_cast<ArrayType>(RetTy))
+ return ATy->getNumElements();
else
return 1;
}
+/// Returns the sub-type a function will return at a given Idx. Should
+/// correspond to the result type of an ExtractValue instruction executed with
+/// just that one Idx (i.e. only top-level structure is considered).
+static Type *getRetComponentType(const Function *F, unsigned Idx) {
+ Type *RetTy = F->getReturnType();
+ assert(!RetTy->isVoidTy() && "void type has no subtype");
+
+ if (StructType *STy = dyn_cast<StructType>(RetTy))
+ return STy->getElementType(Idx);
+ else if (ArrayType *ATy = dyn_cast<ArrayType>(RetTy))
+ return ATy->getElementType();
+ else
+ return RetTy;
+}
+
/// MarkIfNotLive - This checks Use for liveness in LiveValues. If Use is not
/// live, it adds Use to the MaybeLiveUses argument. Returns the determined
/// liveness of Use.
@@ -775,39 +793,29 @@ bool DAE::RemoveDeadStuffFromFunction(Fu
if (RetTy->isVoidTy() || HasLiveReturnedArg) {
NRetTy = RetTy;
} else {
- StructType *STy = dyn_cast<StructType>(RetTy);
- if (STy)
- // Look at each of the original return values individually.
- for (unsigned i = 0; i != RetCount; ++i) {
- RetOrArg Ret = CreateRet(F, i);
- if (LiveValues.erase(Ret)) {
- RetTypes.push_back(STy->getElementType(i));
- NewRetIdxs[i] = RetTypes.size() - 1;
- } else {
- ++NumRetValsEliminated;
- DEBUG(dbgs() << "DAE - Removing return value " << i << " from "
- << F->getName() << "\n");
- }
- }
- else
- // We used to return a single value.
- if (LiveValues.erase(CreateRet(F, 0))) {
- RetTypes.push_back(RetTy);
- NewRetIdxs[0] = 0;
+ // Look at each of the original return values individually.
+ for (unsigned i = 0; i != RetCount; ++i) {
+ RetOrArg Ret = CreateRet(F, i);
+ if (LiveValues.erase(Ret)) {
+ RetTypes.push_back(getRetComponentType(F, i));
+ NewRetIdxs[i] = RetTypes.size() - 1;
} else {
- DEBUG(dbgs() << "DAE - Removing return value from " << F->getName()
- << "\n");
++NumRetValsEliminated;
+ DEBUG(dbgs() << "DAE - Removing return value " << i << " from "
+ << F->getName() << "\n");
+ }
+ }
+ if (RetTypes.size() > 1) {
+ // More than one return type? Reduce it down to size.
+ if (StructType *STy = dyn_cast<StructType>(RetTy)) {
+ // Make the new struct packed if we used to return a packed struct
+ // already.
+ NRetTy = StructType::get(STy->getContext(), RetTypes, STy->isPacked());
+ } else {
+ assert(isa<ArrayType>(RetTy) && "unexpected multi-value return");
+ NRetTy = ArrayType::get(RetTypes[0], RetTypes.size());
}
- if (RetTypes.size() > 1)
- // More than one return type? Return a struct with them. Also, if we used
- // to return a struct and didn't change the number of return values,
- // return a struct again. This prevents changing {something} into
- // something and {} into void.
- // Make the new struct packed if we used to return a packed struct
- // already.
- NRetTy = StructType::get(STy->getContext(), RetTypes, STy->isPacked());
- else if (RetTypes.size() == 1)
+ } else if (RetTypes.size() == 1)
// One return type? Just a simple value then, but only if we didn't use to
// return a struct with that simple value before.
NRetTy = RetTypes.front();
@@ -959,9 +967,9 @@ bool DAE::RemoveDeadStuffFromFunction(Fu
if (!Call->getType()->isX86_MMXTy())
Call->replaceAllUsesWith(Constant::getNullValue(Call->getType()));
} else {
- assert(RetTy->isStructTy() &&
+ assert((RetTy->isStructTy() || RetTy->isArrayTy()) &&
"Return type changed, but not into a void. The old return type"
- " must have been a struct!");
+ " must have been a struct or an array!");
Instruction *InsertPt = Call;
if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
BasicBlock::iterator IP = II->getNormalDest()->begin();
@@ -969,9 +977,9 @@ bool DAE::RemoveDeadStuffFromFunction(Fu
InsertPt = IP;
}
- // 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.
+ // We used to return a struct or array. Instead of doing smart stuff
+ // with all the uses, 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 = UndefValue::get(RetTy);
@@ -1034,8 +1042,8 @@ bool DAE::RemoveDeadStuffFromFunction(Fu
if (NFTy->getReturnType()->isVoidTy()) {
RetVal = nullptr;
} else {
- assert (RetTy->isStructTy());
- // The original return value was a struct, insert
+ assert(RetTy->isStructTy() || RetTy->isArrayTy());
+ // The original return value was a struct or array, insert
// extractvalue/insertvalue chains to extract only the values we need
// to return and insert them into our new result.
// This does generate messy code, but we'll let it to instcombine to
Modified: llvm/trunk/test/Transforms/DeadArgElim/aggregates.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadArgElim/aggregates.ll?rev=228559&r1=228558&r2=228559&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/DeadArgElim/aggregates.ll (original)
+++ llvm/trunk/test/Transforms/DeadArgElim/aggregates.ll Sun Feb 8 19:21:00 2015
@@ -68,4 +68,48 @@ use_aggregate:
ret { i32, i32 } %val
}
-declare void @callee(i32)
\ No newline at end of file
+declare void @callee(i32)
+
+; Case 3: the insertvalue meant %in was live if ret-slot-1 was, but we were only
+; tracking multiple ret-slots for struct types. So %in was eliminated
+; incorrectly.
+
+; CHECK-LABEL: define internal [2 x i32] @array_rets_have_multiple_slots(i32 %in)
+
+define internal [2 x i32] @array_rets_have_multiple_slots(i32 %in) {
+ %ret = insertvalue [2 x i32] undef, i32 %in, 1
+ ret [2 x i32] %ret
+}
+
+define [2 x i32] @test_array_rets_have_multiple_slots() {
+ %res = call [2 x i32] @array_rets_have_multiple_slots(i32 42)
+ ret [2 x i32] %res
+}
+
+; Case 4: we can remove some retvals from the array. It's nice to produce an
+; array again having done so (rather than converting it to a struct).
+
+; CHECK-LABEL: define internal [2 x i32] @can_shrink_arrays()
+; CHECK: [[VAL0:%.*]] = extractvalue [3 x i32] [i32 42, i32 43, i32 44], 0
+; CHECK: [[RESTMP:%.*]] = insertvalue [2 x i32] undef, i32 [[VAL0]], 0
+; CHECK: [[VAL2:%.*]] = extractvalue [3 x i32] [i32 42, i32 43, i32 44], 2
+; CHECK: [[RES:%.*]] = insertvalue [2 x i32] [[RESTMP]], i32 [[VAL2]], 1
+; CHECK: ret [2 x i32] [[RES]]
+
+; CHECK-LABEL: define void @test_can_shrink_arrays()
+
+define internal [3 x i32] @can_shrink_arrays() {
+ ret [3 x i32] [i32 42, i32 43, i32 44]
+}
+
+define void @test_can_shrink_arrays() {
+ %res = call [3 x i32] @can_shrink_arrays()
+
+ %res.0 = extractvalue [3 x i32] %res, 0
+ call void @callee(i32 %res.0)
+
+ %res.2 = extractvalue [3 x i32] %res, 2
+ call void @callee(i32 %res.2)
+
+ ret void
+}
More information about the llvm-commits
mailing list