RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-10-19 19:12:56 +02:00
Code Issues Projects Releases Wiki Activity

DeadArgElim: fix mismatch in accounting of array return types.

Browse Source 
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.

llvm-svn: 228559
This commit is contained in:
Tim Northover 2015-02-09 01:21:00 +00:00
parent 3d150e22ef
commit e34e125ece
2 changed files with 92 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
lib/Transforms/IPO/DeadArgumentElimination.cpp
View File

@ -387,14 +387,32 @@ bool DAE::RemoveDeadArgumentsFromCallers(Function &Fn)
/// 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(Function *F) {
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? 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)
}
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());
}
} 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(Function *F) {
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(Function *F) {
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(Function *F) {
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

46
test/Transforms/DeadArgElim/aggregates.ll
View File

@ -68,4 +68,48 @@ use_aggregate:
ret { i32, i32 } %val
}
declare void @callee(i32)
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
}