<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi Dan,<div>Comments inlined below.</div><div><br><div><div>On Jan 12, 2012, at 4:39 PM, Dan Gohman wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Author: djg<br>Date: Thu Jan 12 18:39:07 2012<br>New Revision: 148076<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=148076&view=rev">http://llvm.org/viewvc/llvm-project?rev=148076&view=rev</a><br>Log:<br>Implement proper ObjC ARC objc_retainBlock "escape" analysis, so that<br>the optimizer doesn't eliminate objc_retainBlock calls which are needed<br>for their side effect of copying blocks onto the heap.<br>This implements <a href="rdar://10361249">rdar://10361249</a>.<br><br>Added:<br> llvm/trunk/test/Transforms/ObjCARC/retain-block.ll<br>Modified:<br> llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp<br> llvm/trunk/test/Transforms/ObjCARC/basic.ll<br><br>Modified: llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp?rev=148076&r1=148075&r2=148076&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp?rev=148076&r1=148075&r2=148076&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp (original)<br>+++ llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp Thu Jan 12 18:39:07 2012<br>@@ -601,6 +601,36 @@<br> M.getNamedValue("objc_unretainedPointer");<br> }<br><br>+/// DoesObjCBlockEscape - Test whether the given pointer, which is an<br>+/// Objective C block pointer, does not "escape". This differs from regular<br>+/// escape analysis in that a use as an argument to a call is not considered<br>+/// an escape.<br>+static bool DoesObjCBlockEscape(const Value *BlockPtr) {<br>+ // Walk the def-use chains.<br>+ SmallVector<const Value *, 4> Worklist;<br>+ Worklist.push_back(BlockPtr);<br>+ do {<br>+ const Value *V = Worklist.pop_back_val();<br>+ for (Value::const_use_iterator UI = V->use_begin(), UE = V->use_end();<br>+ UI != UE; ++UI) {<br>+ const User *UUser = *UI;<br>+ // Special - Use by a call (callee or argument) is not considered<br>+ // to be an escape.<br>+ if (ImmutableCallSite CS = cast<Value>(UUser))<br>+ continue;<br></div></blockquote><div><br></div><div>I see:</div><div><br></div><div><span class="Apple-style-span" style="color: rgb(255, 0, 0); font-family: 'Courier New', courier, monotype; white-space: pre; ">ObjCARC.cpp:619:29: warning: unused variable 'CS' [-Wunused-variable]</span></div><div><span class="Apple-style-span" style="font-family: Times; "><pre style="font-family: 'Courier New', courier, monotype; "><span class="stderr" style="font-family: 'Courier New', courier, monotype; color: red; "> if (ImmutableCallSite CS = cast<Value>(UUser))
</span></pre></span></div><br><blockquote type="cite"><div>+ if (isa<BitCastInst>(UUser) || isa<GetElementPtrInst>(UUser) ||<br>+ isa<PHINode>(UUser) || isa<SelectInst>(UUser)) {<br>+ Worklist.push_back(UUser);<br>+ continue;<br>+ }<br>+ return true;<br>+ }<br>+ } while (!Worklist.empty());<br>+<br>+ // No escapes found.<br>+ return false;<br>+}<br>+<br> //===----------------------------------------------------------------------===//<br> // ARC AliasAnalysis.<br> //===----------------------------------------------------------------------===//<br>@@ -1159,10 +1189,6 @@<br> /// opposed to objc_retain calls).<br> bool IsRetainBlock;<br><br>- /// CopyOnEscape - True if this the Calls are objc_retainBlock calls<br>- /// which all have the !clang.arc.copy_on_escape metadata.<br>- bool CopyOnEscape;<br>-<br> /// IsTailCallRelease - True of the objc_release calls are all marked<br> /// with the "tail" keyword.<br> bool IsTailCallRelease;<br>@@ -1186,7 +1212,7 @@<br> SmallPtrSet<Instruction *, 2> ReverseInsertPts;<br><br> RRInfo() :<br>- KnownSafe(false), IsRetainBlock(false), CopyOnEscape(false),<br>+ KnownSafe(false), IsRetainBlock(false),<br> IsTailCallRelease(false), Partial(false),<br> ReleaseMetadata(0) {}<br><br>@@ -1197,7 +1223,6 @@<br> void RRInfo::clear() {<br> KnownSafe = false;<br> IsRetainBlock = false;<br>- CopyOnEscape = false;<br> IsTailCallRelease = false;<br> Partial = false;<br> ReleaseMetadata = 0;<br>@@ -1295,7 +1320,6 @@<br> if (RRI.ReleaseMetadata != Other.RRI.ReleaseMetadata)<br> RRI.ReleaseMetadata = 0;<br><br>- RRI.CopyOnEscape = RRI.CopyOnEscape && Other.RRI.CopyOnEscape;<br> RRI.KnownSafe = RRI.KnownSafe && Other.RRI.KnownSafe;<br> RRI.IsTailCallRelease = RRI.IsTailCallRelease && Other.RRI.IsTailCallRelease;<br> RRI.Calls.insert(Other.RRI.Calls.begin(), Other.RRI.Calls.end());<br>@@ -1495,6 +1519,8 @@<br> Constant *getRetainBlockCallee(Module *M);<br> Constant *getAutoreleaseCallee(Module *M);<br><br>+ bool IsRetainBlockOptimizable(const Instruction *Inst);<br>+<br> void OptimizeRetainCall(Function &F, Instruction *Retain);<br> bool OptimizeRetainRVCall(Function &F, Instruction *RetainRV);<br> void OptimizeAutoreleaseRVCall(Function &F, Instruction *AutoreleaseRV);<br>@@ -1562,6 +1588,22 @@<br> AU.setPreservesCFG();<br> }<br><br>+bool ObjCARCOpt::IsRetainBlockOptimizable(const Instruction *Inst) {<br>+ // Without the magic metadata tag, we have to assume this might be an<br>+ // objc_retainBlock call inserted to convert a block pointer to an id,<br>+ // in which case it really is needed.<br>+ if (!Inst->getMetadata(CopyOnEscapeMDKind))<br>+ return false;<br>+<br>+ // If the pointer "escapes" (not including being used in a call),<br>+ // the copy may be needed.<br>+ if (DoesObjCBlockEscape(Inst))<br>+ return false;<br>+<br>+ // Otherwise, it's not needed.<br>+ return true;<br>+}<br>+<br> Constant *ObjCARCOpt::getRetainRVCallee(Module *M) {<br> if (!RetainRVCallee) {<br> LLVMContext &C = M->getContext();<br>@@ -2362,6 +2404,11 @@<br> break;<br> }<br> case IC_RetainBlock:<br>+ // An objc_retainBlock call with just a use may need to be kept,<br>+ // because it may be copying a block from the stack to the heap.<br>+ if (!IsRetainBlockOptimizable(Inst))<br>+ break;<br>+ // FALLTHROUGH<br> case IC_Retain:<br> case IC_RetainRV: {<br> Arg = GetObjCArg(Inst);<br>@@ -2371,14 +2418,6 @@<br> S.SetAtLeastOneRefCount();<br> S.DecrementNestCount();<br><br>- // An non-copy-on-escape objc_retainBlock call with just a use still<br>- // needs to be kept, because it may be copying a block from the stack<br>- // to the heap.<br>- if (Class == IC_RetainBlock &&<br>- !Inst->getMetadata(CopyOnEscapeMDKind) &&<br>- S.GetSeq() == S_Use)<br>- S.SetSeq(S_CanRelease);<br>-<br> switch (S.GetSeq()) {<br> case S_Stop:<br> case S_Release:<br>@@ -2391,8 +2430,6 @@<br> // better to let it remain as the first instruction after a call.<br> if (Class != IC_RetainRV) {<br> S.RRI.IsRetainBlock = Class == IC_RetainBlock;<br>- if (S.RRI.IsRetainBlock)<br>- S.RRI.CopyOnEscape = !!Inst->getMetadata(CopyOnEscapeMDKind);<br> Retains[Inst] = S.RRI;<br> }<br> S.ClearSequenceProgress();<br>@@ -2519,6 +2556,11 @@<br><br> switch (Class) {<br> case IC_RetainBlock:<br>+ // An objc_retainBlock call with just a use may need to be kept,<br>+ // because it may be copying a block from the stack to the heap.<br>+ if (!IsRetainBlockOptimizable(Inst))<br>+ break;<br>+ // FALLTHROUGH<br> case IC_Retain:<br> case IC_RetainRV: {<br> Arg = GetObjCArg(Inst);<br>@@ -2541,8 +2583,6 @@<br> S.SetSeq(S_Retain);<br> S.RRI.clear();<br> S.RRI.IsRetainBlock = Class == IC_RetainBlock;<br>- if (S.RRI.IsRetainBlock)<br>- S.RRI.CopyOnEscape = !!Inst->getMetadata(CopyOnEscapeMDKind);<br> // Don't check S.IsKnownIncremented() here because it's not<br> // sufficient.<br> S.RRI.KnownSafe = S.IsKnownNested();<br>@@ -2634,17 +2674,6 @@<br> S.SetSeq(S_Use);<br> break;<br> case S_Retain:<br>- // A non-copy-on-scape objc_retainBlock call may be responsible for<br>- // copying the block data from the stack to the heap. Model this by<br>- // moving it straight from S_Retain to S_Use.<br>- if (S.RRI.IsRetainBlock &&<br>- !S.RRI.CopyOnEscape &&<br>- CanUse(Inst, Ptr, PA, Class)) {<br>- assert(S.RRI.ReverseInsertPts.empty());<br>- S.RRI.ReverseInsertPts.insert(Inst);<br>- S.SetSeq(S_Use);<br>- }<br>- break;<br> case S_Use:<br> case S_None:<br> break;<br>@@ -2787,10 +2816,10 @@<br> getRetainBlockCallee(M) : getRetainCallee(M),<br> MyArg, "", InsertPt);<br> Call->setDoesNotThrow();<br>- if (RetainsToMove.CopyOnEscape)<br>+ if (RetainsToMove.IsRetainBlock)<br> Call->setMetadata(CopyOnEscapeMDKind,<br> MDNode::get(M->getContext(), ArrayRef<Value *>()));<br>- if (!RetainsToMove.IsRetainBlock)<br>+ else<br> Call->setTailCall();<br> }<br> for (SmallPtrSet<Instruction *, 2>::const_iterator<br>@@ -2864,18 +2893,11 @@<br> Instruction *Retain = cast<Instruction>(V);<br> Value *Arg = GetObjCArg(Retain);<br><br>- // If the object being released is in static storage, we know it's<br>+ // If the object being released is in static or stack storage, we know it's<br> // not being managed by ObjC reference counting, so we can delete pairs<br> // regardless of what possible decrements or uses lie between them.<br>- bool KnownSafe = isa<Constant>(Arg);<br>+ bool KnownSafe = isa<Constant>(Arg) || isa<AllocaInst>(Arg);<br><br>- // Same for stack storage, unless this is a non-copy-on-escape<br>- // objc_retainBlock call, which is responsible for copying the block data<br>- // from the stack to the heap.<br>- if ((!I->second.IsRetainBlock || I->second.CopyOnEscape) &&<br>- isa<AllocaInst>(Arg))<br>- KnownSafe = true;<br>-<br> // A constant pointer can't be pointing to an object on the heap. It may<br> // be reference-counted, but it won't be deleted.<br> if (const LoadInst *LI = dyn_cast<LoadInst>(Arg))<br>@@ -2983,7 +3005,6 @@<br> // Merge the IsRetainBlock values.<br> if (FirstRetain) {<br> RetainsToMove.IsRetainBlock = NewReleaseRetainRRI.IsRetainBlock;<br>- RetainsToMove.CopyOnEscape = NewReleaseRetainRRI.CopyOnEscape;<br> FirstRetain = false;<br> } else if (ReleasesToMove.IsRetainBlock !=<br> NewReleaseRetainRRI.IsRetainBlock)<br>@@ -2991,9 +3012,6 @@<br> // objc_retain and the other uses objc_retainBlock.<br> goto next_retain;<br><br>- // Merge the CopyOnEscape values.<br>- RetainsToMove.CopyOnEscape &= NewReleaseRetainRRI.CopyOnEscape;<br>-<br> // Collect the optimal insertion points.<br> if (!KnownSafe)<br> for (SmallPtrSet<Instruction *, 2>::const_iterator<br><br>Modified: llvm/trunk/test/Transforms/ObjCARC/basic.ll<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/basic.ll?rev=148076&r1=148075&r2=148076&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/basic.ll?rev=148076&r1=148075&r2=148076&view=diff</a><br>==============================================================================<br>--- llvm/trunk/test/Transforms/ObjCARC/basic.ll (original)<br>+++ llvm/trunk/test/Transforms/ObjCARC/basic.ll Thu Jan 12 18:39:07 2012<br>@@ -786,7 +786,7 @@<br> @__block_holder_tmp_1 = external constant %block1<br> define void @test23() {<br> entry:<br>- %0 = call i8* @objc_retainBlock(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind<br>+ %0 = call i8* @objc_retainBlock(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind, !clang.arc.copy_on_escape !0<br> call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))<br> call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))<br> call void @objc_release(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind<br>@@ -801,13 +801,28 @@<br> ; CHECK: }<br> define void @test23b(i8* %p) {<br> entry:<br>- %0 = call i8* @objc_retainBlock(i8* %p) nounwind<br>+ %0 = call i8* @objc_retainBlock(i8* %p) nounwind, !clang.arc.copy_on_escape !0<br> call void @callee()<br> call void @use_pointer(i8* %p)<br> call void @objc_release(i8* %p) nounwind<br> ret void<br> }<br><br>+; Don't optimize objc_retainBlock, because there's no copy_on_escape metadata.<br>+<br>+; CHECK: define void @test23c(<br>+; CHECK: @objc_retainBlock<br>+; CHECK: @objc_release<br>+; CHECK: }<br>+define void @test23c() {<br>+entry:<br>+ %0 = call i8* @objc_retainBlock(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind<br>+ call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))<br>+ call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))<br>+ call void @objc_release(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind<br>+ ret void<br>+}<br>+<br> ; Any call can decrement a retain count.<br><br> ; CHECK: define void @test24(<br><br>Added: llvm/trunk/test/Transforms/ObjCARC/retain-block.ll<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/retain-block.ll?rev=148076&view=auto">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/retain-block.ll?rev=148076&view=auto</a><br>==============================================================================<br>--- llvm/trunk/test/Transforms/ObjCARC/retain-block.ll (added)<br>+++ llvm/trunk/test/Transforms/ObjCARC/retain-block.ll Thu Jan 12 18:39:07 2012<br>@@ -0,0 +1,138 @@<br>+; RUN: opt -objc-arc -S < %s | FileCheck %s<br>+<br>+target datalayout = "e-p:64:64:64"<br>+<br>+!0 = metadata !{}<br>+<br>+declare i8* @objc_retain(i8*)<br>+declare void @callee(i8)<br>+declare void @use_pointer(i8*)<br>+declare void @objc_release(i8*)<br>+declare i8* @objc_retainBlock(i8*)<br>+declare i8* @objc_autorelease(i8*)<br>+<br>+; Basic retainBlock+release elimination.<br>+<br>+; CHECK: define void @test0(i8* %tmp) {<br>+; CHECK-NOT: @objc<br>+; CHECK: }<br>+define void @test0(i8* %tmp) {<br>+entry:<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br>+<br>+; Same as test0, but there's no copy_on_escape metadata, so there's no<br>+; optimization possible.<br>+<br>+; CHECK: define void @test0_no_metadata(i8* %tmp) {<br>+; CHECK: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind<br>+; CHECK: tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+; CHECK: }<br>+define void @test0_no_metadata(i8* %tmp) {<br>+entry:<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br>+<br>+; Same as test0, but the pointer escapes, so there's no<br>+; optimization possible.<br>+<br>+; CHECK: define void @test0_escape(i8* %tmp, i8** %z) {<br>+; CHECK: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+; CHECK: tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+; CHECK: }<br>+define void @test0_escape(i8* %tmp, i8** %z) {<br>+entry:<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+ store i8* %tmp2, i8** %z<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br>+<br>+; Same as test0_escape, but there's no intervening call.<br>+<br>+; CHECK: define void @test0_just_escape(i8* %tmp, i8** %z) {<br>+; CHECK: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+; CHECK: tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+; CHECK: }<br>+define void @test0_just_escape(i8* %tmp, i8** %z) {<br>+entry:<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+ store i8* %tmp2, i8** %z<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br>+<br>+; Basic nested retainBlock+release elimination.<br>+<br>+; CHECK: define void @test1(i8* %tmp) {<br>+; CHECK-NOT: @objc<br>+; CHECK: tail call i8* @objc_retain(i8* %tmp) nounwind<br>+; CHECK-NOT: @objc<br>+; CHECK: tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0<br>+; CHECK-NOT: @objc<br>+; CHECK: }<br>+define void @test1(i8* %tmp) {<br>+entry:<br>+ %tmp1 = tail call i8* @objc_retain(i8* %tmp) nounwind<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br>+<br>+; Same as test1, but there's no copy_on_escape metadata, so there's no<br>+; retainBlock+release optimization possible. But we can still eliminate<br>+; the outer retain+release.<br>+<br>+; CHECK: define void @test1_no_metadata(i8* %tmp) {<br>+; CHECK-NEXT: entry:<br>+; CHECK-NEXT: tail call i8* @objc_retainBlock(i8* %tmp) nounwind<br>+; CHECK-NEXT: @use_pointer(i8* %tmp2)<br>+; CHECK-NEXT: @use_pointer(i8* %tmp2)<br>+; CHECK-NEXT: tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0<br>+; CHECK-NOT: @objc<br>+; CHECK: }<br>+define void @test1_no_metadata(i8* %tmp) {<br>+entry:<br>+ %tmp1 = tail call i8* @objc_retain(i8* %tmp) nounwind<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br>+<br>+; Same as test1, but the pointer escapes, so there's no<br>+; retainBlock+release optimization possible. But we can still eliminate<br>+; the outer retain+release<br>+<br>+; CHECK: define void @test1_escape(i8* %tmp, i8** %z) {<br>+; CHECK-NEXT: entry:<br>+; CHECK-NEXT: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+; CHECK-NEXT: store i8* %tmp2, i8** %z<br>+; CHECK-NEXT: @use_pointer(i8* %tmp2)<br>+; CHECK-NEXT: @use_pointer(i8* %tmp2)<br>+; CHECK-NEXT: tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0<br>+; CHECK-NOT: @objc<br>+; CHECK: }<br>+define void @test1_escape(i8* %tmp, i8** %z) {<br>+entry:<br>+ %tmp1 = tail call i8* @objc_retain(i8* %tmp) nounwind<br>+ %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0<br>+ store i8* %tmp2, i8** %z<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @use_pointer(i8* %tmp2)<br>+ tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0<br>+ tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0<br>+ ret void<br>+}<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></div></blockquote></div><br></div></body></html>