From a254acd1d32f254a570a9826d779ebf0bf57e7ee Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Wed, 7 Jan 2009 19:39:06 +0000 Subject: [PATCH] Remove alloca tracking from nocapture analysis. Not only was it not very helpful, it was also wrong! The problem is shown in the testcase: the alloca might be passed to a nocapture callee which dereferences it and returns the original pointer. But because it was a nocapture call we think we don't need to track its uses, but we do. llvm-svn: 61876 --- lib/Transforms/IPO/FunctionAttrs.cpp | 85 ++++--------------- .../FunctionAttrs/2009-01-02-LocalStores.ll | 29 +++---- 2 files changed, 26 insertions(+), 88 deletions(-) diff --git a/lib/Transforms/IPO/FunctionAttrs.cpp b/lib/Transforms/IPO/FunctionAttrs.cpp index 877c7c9b507..e29e9149c26 100644 --- a/lib/Transforms/IPO/FunctionAttrs.cpp +++ b/lib/Transforms/IPO/FunctionAttrs.cpp @@ -183,32 +183,20 @@ bool FunctionAttrs::AddReadAttrs(const std::vector &SCC) { /// isCaptured - Return true if this pointer value may be captured. bool FunctionAttrs::isCaptured(Function &F, Value *V) { - typedef PointerIntPair UseWithDepth; - SmallVector Worklist; - SmallSet Visited; + SmallVector Worklist; + SmallSet Visited; for (Value::use_iterator UI = V->use_begin(), UE = V->use_end(); UI != UE; ++UI) { - UseWithDepth UD(&UI.getUse(), 0); - Visited.insert(UD); - Worklist.push_back(UD); + Use *U = &UI.getUse(); + Visited.insert(U); + Worklist.push_back(U); } while (!Worklist.empty()) { - UseWithDepth UD = Worklist.pop_back_val(); - Use *U = UD.getPointer(); + Use *U = Worklist.pop_back_val(); Instruction *I = cast(U->getUser()); - // The value V may have any type if it comes from tracking a load. V = U->get(); - // The depth represents the number of loads that need to be performed to - // get back the original pointer (or a bitcast etc of it). For example, - // if the pointer is stored to an alloca, then all uses of the alloca get - // depth 1: if the alloca is loaded then you get the original pointer back. - // If a load of the alloca is returned then the pointer has been captured. - // The depth is needed in order to know which loads dereference the original - // pointer (these do not capture), and which return a value which needs to - // be tracked because if it is captured then so is the original pointer. - unsigned Depth = UD.getInt(); switch (I->getOpcode()) { case Instruction::Call: @@ -238,66 +226,25 @@ bool FunctionAttrs::isCaptured(Function &F, Value *V) { case Instruction::Free: // Freeing a pointer does not cause it to be captured. break; + case Instruction::Load: + // Loading from a pointer does not cause it to be captured. + break; case Instruction::Store: - if (V == I->getOperand(0)) { - // Stored the pointer - it may be captured. If it is stored to a local - // object (alloca) then track that object. Otherwise give up. - Value *Target = I->getOperand(1)->getUnderlyingObject(); - if (!isa(Target)) - // Didn't store to an obviously local object - captured. - return true; - if (Depth >= 3) - // Alloca recursion too deep - give up. - return true; - // Analyze all uses of the alloca. - for (Value::use_iterator UI = Target->use_begin(), - UE = Target->use_end(); UI != UE; ++UI) { - UseWithDepth NUD(&UI.getUse(), Depth + 1); - if (Visited.insert(NUD)) - Worklist.push_back(NUD); - } - } + if (V == I->getOperand(0)) + // Stored the pointer - it may be captured. + return true; // Storing to the pointee does not cause the pointer to be captured. break; case Instruction::BitCast: case Instruction::GetElementPtr: - case Instruction::Load: case Instruction::PHI: case Instruction::Select: - // Track any uses of this instruction to see if they are captured. - // First handle any special cases. - if (isa(I)) { - // Play safe and do not accept being used as an index. - if (V != I->getOperand(0)) - return true; - } else if (isa(I)) { - // Play safe and do not accept being used as the condition. - if (V == I->getOperand(0)) - return true; - } else if (isa(I)) { - // Usually loads can be ignored because they dereference the original - // pointer. However the loaded value needs to be tracked if loading - // from an object that the original pointer was stored to. - if (Depth == 0) - // Loading the original pointer or a variation of it. This does not - // cause the pointer to be captured. Note that the loaded value might - // be the pointer itself (think of self-referential objects), but that - // is fine as long as it's not this function that stored it there. - break; - // Loading a pointer to (a pointer to...) the original pointer or a - // variation of it. Track uses of the loaded value, noting that one - // dereference was performed. Note that the loaded value need not be - // of pointer type. For example, an alloca may have been bitcast to - // a pointer to another type, which was then loaded. - --Depth; - } - - // The original value is not captured via this if the instruction isn't. + // The original value is not captured via this if the new value isn't. for (Instruction::use_iterator UI = I->use_begin(), UE = I->use_end(); UI != UE; ++UI) { - UseWithDepth UD(&UI.getUse(), Depth); - if (Visited.insert(UD)) - Worklist.push_back(UD); + Use *U = &UI.getUse(); + if (Visited.insert(U)) + Worklist.push_back(U); } break; default: diff --git a/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll b/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll index 17b443980b7..68a232f5ff3 100644 --- a/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll +++ b/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll @@ -1,23 +1,14 @@ ; RUN: llvm-as < %s | opt -functionattrs | llvm-dis | not grep {nocapture *%%q} ; RUN: llvm-as < %s | opt -functionattrs | llvm-dis | grep {nocapture *%%p} -@g = external global i32** - -define i32 @f(i32* %p, i32* %q) { - %a1 = alloca i32* - %a2 = alloca i32** - store i32* %p, i32** %a1 - store i32** %a1, i32*** %a2 - %reload1 = load i32*** %a2 - %reload2 = load i32** %reload1 - %load_p = load i32* %reload2 - store i32 0, i32* %reload2 - - %b1 = alloca i32* - %b2 = alloca i32** - store i32* %q, i32** %b1 - store i32** %b1, i32*** %b2 - %reload3 = load i32*** %b2 - store i32** %reload3, i32*** @g - ret i32 %load_p +define i32* @a(i32** %p) { + %tmp = load i32** %p + ret i32* %tmp +} + +define i32* @b(i32 *%q) { + %mem = alloca i32* + store i32* %q, i32** %mem + %tmp = call i32* @a(i32** %mem) + ret i32* %tmp }