From df1d2b02f9ac1a00a5a00d7f7ea617b6ac841e34 Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Mon, 25 Feb 2008 04:08:09 +0000 Subject: Fix an issue where GVN was performing the return slot optimization when it was not safe. This is fixed by more aggressively checking that the return slot is not used elsewhere in the function. llvm-svn: 47544 --- llvm/lib/Transforms/Scalar/GVN.cpp | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) (limited to 'llvm/lib') diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index 650612144ad..4b7e82c927e 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -752,6 +752,8 @@ namespace { bool iterateOnFunction(Function &F); Value* CollapsePhi(PHINode* p); bool isSafeReplacement(PHINode* p, Instruction* inst); + bool valueHasOnlyOneUseAfter(Value* val, MemCpyInst* use, + Instruction* cutoff); }; char GVN::ID = 0; @@ -1055,22 +1057,32 @@ bool GVN::processLoad(LoadInst* L, return deletedLoad; } -/// isReturnSlotOptznProfitable - Determine if performing a return slot -/// fusion with the slot dest is profitable -static bool isReturnSlotOptznProfitable(Value* dest, MemCpyInst* cpy) { - // We currently consider it profitable if dest is otherwise dead. - SmallVector useList(dest->use_begin(), dest->use_end()); +/// valueHasOnlyOneUse - Returns true if a value has only one use after the +/// cutoff that is in the current same block and is the same as the use +/// parameter. +bool GVN::valueHasOnlyOneUseAfter(Value* val, MemCpyInst* use, + Instruction* cutoff) { + DominatorTree& DT = getAnalysis(); + + SmallVector useList(val->use_begin(), val->use_end()); while (!useList.empty()) { User* UI = useList.back(); + if (isa(UI) || isa(UI)) { useList.pop_back(); for (User::use_iterator I = UI->use_begin(), E = UI->use_end(); I != E; ++I) useList.push_back(*I); - } else if (UI == cpy) + } else if (UI == use) { useList.pop_back(); - else + } else if (Instruction* inst = dyn_cast(UI)) { + if (inst->getParent() == use->getParent() && + (inst == cutoff || !DT.dominates(cutoff, inst))) { + useList.pop_back(); + } else + return false; + } else return false; } @@ -1123,8 +1135,14 @@ bool GVN::performReturnSlotOptzn(MemCpyInst* cpy, CallInst* C, if (TD.getTypeStoreSize(PT->getElementType()) != cpyLength->getZExtValue()) return false; + // For safety, we must ensure that the output parameter of the call only has + // a single use, the memcpy. Otherwise this can introduce an invalid + // transformation. + if (!valueHasOnlyOneUseAfter(CS.getArgument(0), cpy, C)) + return false; + // We only perform the transformation if it will be profitable. - if (!isReturnSlotOptznProfitable(cpyDest, cpy)) + if (!valueHasOnlyOneUseAfter(cpyDest, cpy, C)) return false; // In addition to knowing that the call does not access the return slot -- cgit v1.2.3