[llvm-commits] [llvm] r148076 - in /llvm/trunk: lib/Transforms/Scalar/ObjCARC.cpp test/Transforms/ObjCARC/basic.ll test/Transforms/ObjCARC/retain-block.ll

Chad Rosier mcrosier at apple.com
Fri Jan 13 12:13:58 PST 2012


Hi Dan,
Comments inlined below.

On Jan 12, 2012, at 4:39 PM, Dan Gohman wrote:

> Author: djg
> Date: Thu Jan 12 18:39:07 2012
> New Revision: 148076
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=148076&view=rev
> Log:
> Implement proper ObjC ARC objc_retainBlock "escape" analysis, so that
> the optimizer doesn't eliminate objc_retainBlock calls which are needed
> for their side effect of copying blocks onto the heap.
> This implements rdar://10361249.
> 
> Added:
>    llvm/trunk/test/Transforms/ObjCARC/retain-block.ll
> Modified:
>    llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp
>    llvm/trunk/test/Transforms/ObjCARC/basic.ll
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp?rev=148076&r1=148075&r2=148076&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp Thu Jan 12 18:39:07 2012
> @@ -601,6 +601,36 @@
>     M.getNamedValue("objc_unretainedPointer");
> }
> 
> +/// DoesObjCBlockEscape - Test whether the given pointer, which is an
> +/// Objective C block pointer, does not "escape". This differs from regular
> +/// escape analysis in that a use as an argument to a call is not considered
> +/// an escape.
> +static bool DoesObjCBlockEscape(const Value *BlockPtr) {
> +  // Walk the def-use chains.
> +  SmallVector<const Value *, 4> Worklist;
> +  Worklist.push_back(BlockPtr);
> +  do {
> +    const Value *V = Worklist.pop_back_val();
> +    for (Value::const_use_iterator UI = V->use_begin(), UE = V->use_end();
> +         UI != UE; ++UI) {
> +      const User *UUser = *UI;
> +      // Special - Use by a call (callee or argument) is not considered
> +      // to be an escape.
> +      if (ImmutableCallSite CS = cast<Value>(UUser))
> +        continue;

I see:

ObjCARC.cpp:619:29: warning: unused variable 'CS' [-Wunused-variable]
      if (ImmutableCallSite CS = cast<Value>(UUser))

> +      if (isa<BitCastInst>(UUser) || isa<GetElementPtrInst>(UUser) ||
> +          isa<PHINode>(UUser) || isa<SelectInst>(UUser)) {
> +        Worklist.push_back(UUser);
> +        continue;
> +      }
> +      return true;
> +    }
> +  } while (!Worklist.empty());
> +
> +  // No escapes found.
> +  return false;
> +}
> +
> //===----------------------------------------------------------------------===//
> // ARC AliasAnalysis.
> //===----------------------------------------------------------------------===//
> @@ -1159,10 +1189,6 @@
>     /// opposed to objc_retain calls).
>     bool IsRetainBlock;
> 
> -    /// CopyOnEscape - True if this the Calls are objc_retainBlock calls
> -    /// which all have the !clang.arc.copy_on_escape metadata.
> -    bool CopyOnEscape;
> -
>     /// IsTailCallRelease - True of the objc_release calls are all marked
>     /// with the "tail" keyword.
>     bool IsTailCallRelease;
> @@ -1186,7 +1212,7 @@
>     SmallPtrSet<Instruction *, 2> ReverseInsertPts;
> 
>     RRInfo() :
> -      KnownSafe(false), IsRetainBlock(false), CopyOnEscape(false),
> +      KnownSafe(false), IsRetainBlock(false),
>       IsTailCallRelease(false), Partial(false),
>       ReleaseMetadata(0) {}
> 
> @@ -1197,7 +1223,6 @@
> void RRInfo::clear() {
>   KnownSafe = false;
>   IsRetainBlock = false;
> -  CopyOnEscape = false;
>   IsTailCallRelease = false;
>   Partial = false;
>   ReleaseMetadata = 0;
> @@ -1295,7 +1320,6 @@
>     if (RRI.ReleaseMetadata != Other.RRI.ReleaseMetadata)
>       RRI.ReleaseMetadata = 0;
> 
> -    RRI.CopyOnEscape = RRI.CopyOnEscape && Other.RRI.CopyOnEscape;
>     RRI.KnownSafe = RRI.KnownSafe && Other.RRI.KnownSafe;
>     RRI.IsTailCallRelease = RRI.IsTailCallRelease && Other.RRI.IsTailCallRelease;
>     RRI.Calls.insert(Other.RRI.Calls.begin(), Other.RRI.Calls.end());
> @@ -1495,6 +1519,8 @@
>     Constant *getRetainBlockCallee(Module *M);
>     Constant *getAutoreleaseCallee(Module *M);
> 
> +    bool IsRetainBlockOptimizable(const Instruction *Inst);
> +
>     void OptimizeRetainCall(Function &F, Instruction *Retain);
>     bool OptimizeRetainRVCall(Function &F, Instruction *RetainRV);
>     void OptimizeAutoreleaseRVCall(Function &F, Instruction *AutoreleaseRV);
> @@ -1562,6 +1588,22 @@
>   AU.setPreservesCFG();
> }
> 
> +bool ObjCARCOpt::IsRetainBlockOptimizable(const Instruction *Inst) {
> +  // Without the magic metadata tag, we have to assume this might be an
> +  // objc_retainBlock call inserted to convert a block pointer to an id,
> +  // in which case it really is needed.
> +  if (!Inst->getMetadata(CopyOnEscapeMDKind))
> +    return false;
> +
> +  // If the pointer "escapes" (not including being used in a call),
> +  // the copy may be needed.
> +  if (DoesObjCBlockEscape(Inst))
> +    return false;
> +
> +  // Otherwise, it's not needed.
> +  return true;
> +}
> +
> Constant *ObjCARCOpt::getRetainRVCallee(Module *M) {
>   if (!RetainRVCallee) {
>     LLVMContext &C = M->getContext();
> @@ -2362,6 +2404,11 @@
>       break;
>     }
>     case IC_RetainBlock:
> +      // An objc_retainBlock call with just a use may need to be kept,
> +      // because it may be copying a block from the stack to the heap.
> +      if (!IsRetainBlockOptimizable(Inst))
> +        break;
> +      // FALLTHROUGH
>     case IC_Retain:
>     case IC_RetainRV: {
>       Arg = GetObjCArg(Inst);
> @@ -2371,14 +2418,6 @@
>       S.SetAtLeastOneRefCount();
>       S.DecrementNestCount();
> 
> -      // An non-copy-on-escape objc_retainBlock call with just a use still
> -      // needs to be kept, because it may be copying a block from the stack
> -      // to the heap.
> -      if (Class == IC_RetainBlock &&
> -          !Inst->getMetadata(CopyOnEscapeMDKind) &&
> -          S.GetSeq() == S_Use)
> -        S.SetSeq(S_CanRelease);
> -
>       switch (S.GetSeq()) {
>       case S_Stop:
>       case S_Release:
> @@ -2391,8 +2430,6 @@
>         // better to let it remain as the first instruction after a call.
>         if (Class != IC_RetainRV) {
>           S.RRI.IsRetainBlock = Class == IC_RetainBlock;
> -          if (S.RRI.IsRetainBlock)
> -            S.RRI.CopyOnEscape = !!Inst->getMetadata(CopyOnEscapeMDKind);
>           Retains[Inst] = S.RRI;
>         }
>         S.ClearSequenceProgress();
> @@ -2519,6 +2556,11 @@
> 
>     switch (Class) {
>     case IC_RetainBlock:
> +      // An objc_retainBlock call with just a use may need to be kept,
> +      // because it may be copying a block from the stack to the heap.
> +      if (!IsRetainBlockOptimizable(Inst))
> +        break;
> +      // FALLTHROUGH
>     case IC_Retain:
>     case IC_RetainRV: {
>       Arg = GetObjCArg(Inst);
> @@ -2541,8 +2583,6 @@
>         S.SetSeq(S_Retain);
>         S.RRI.clear();
>         S.RRI.IsRetainBlock = Class == IC_RetainBlock;
> -        if (S.RRI.IsRetainBlock)
> -          S.RRI.CopyOnEscape = !!Inst->getMetadata(CopyOnEscapeMDKind);
>         // Don't check S.IsKnownIncremented() here because it's not
>         // sufficient.
>         S.RRI.KnownSafe = S.IsKnownNested();
> @@ -2634,17 +2674,6 @@
>           S.SetSeq(S_Use);
>         break;
>       case S_Retain:
> -        // A non-copy-on-scape objc_retainBlock call may be responsible for
> -        // copying the block data from the stack to the heap. Model this by
> -        // moving it straight from S_Retain to S_Use.
> -        if (S.RRI.IsRetainBlock &&
> -            !S.RRI.CopyOnEscape &&
> -            CanUse(Inst, Ptr, PA, Class)) {
> -          assert(S.RRI.ReverseInsertPts.empty());
> -          S.RRI.ReverseInsertPts.insert(Inst);
> -          S.SetSeq(S_Use);
> -        }
> -        break;
>       case S_Use:
>       case S_None:
>         break;
> @@ -2787,10 +2816,10 @@
>                          getRetainBlockCallee(M) : getRetainCallee(M),
>                        MyArg, "", InsertPt);
>     Call->setDoesNotThrow();
> -    if (RetainsToMove.CopyOnEscape)
> +    if (RetainsToMove.IsRetainBlock)
>       Call->setMetadata(CopyOnEscapeMDKind,
>                         MDNode::get(M->getContext(), ArrayRef<Value *>()));
> -    if (!RetainsToMove.IsRetainBlock)
> +    else
>       Call->setTailCall();
>   }
>   for (SmallPtrSet<Instruction *, 2>::const_iterator
> @@ -2864,18 +2893,11 @@
>     Instruction *Retain = cast<Instruction>(V);
>     Value *Arg = GetObjCArg(Retain);
> 
> -    // If the object being released is in static storage, we know it's
> +    // If the object being released is in static or stack storage, we know it's
>     // not being managed by ObjC reference counting, so we can delete pairs
>     // regardless of what possible decrements or uses lie between them.
> -    bool KnownSafe = isa<Constant>(Arg);
> +    bool KnownSafe = isa<Constant>(Arg) || isa<AllocaInst>(Arg);
> 
> -    // Same for stack storage, unless this is a non-copy-on-escape
> -    // objc_retainBlock call, which is responsible for copying the block data
> -    // from the stack to the heap.
> -    if ((!I->second.IsRetainBlock || I->second.CopyOnEscape) &&
> -        isa<AllocaInst>(Arg))
> -      KnownSafe = true;
> -
>     // A constant pointer can't be pointing to an object on the heap. It may
>     // be reference-counted, but it won't be deleted.
>     if (const LoadInst *LI = dyn_cast<LoadInst>(Arg))
> @@ -2983,7 +3005,6 @@
>             // Merge the IsRetainBlock values.
>             if (FirstRetain) {
>               RetainsToMove.IsRetainBlock = NewReleaseRetainRRI.IsRetainBlock;
> -              RetainsToMove.CopyOnEscape = NewReleaseRetainRRI.CopyOnEscape;
>               FirstRetain = false;
>             } else if (ReleasesToMove.IsRetainBlock !=
>                        NewReleaseRetainRRI.IsRetainBlock)
> @@ -2991,9 +3012,6 @@
>               // objc_retain and the other uses objc_retainBlock.
>               goto next_retain;
> 
> -            // Merge the CopyOnEscape values.
> -            RetainsToMove.CopyOnEscape &= NewReleaseRetainRRI.CopyOnEscape;
> -
>             // Collect the optimal insertion points.
>             if (!KnownSafe)
>               for (SmallPtrSet<Instruction *, 2>::const_iterator
> 
> Modified: llvm/trunk/test/Transforms/ObjCARC/basic.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/basic.ll?rev=148076&r1=148075&r2=148076&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/ObjCARC/basic.ll (original)
> +++ llvm/trunk/test/Transforms/ObjCARC/basic.ll Thu Jan 12 18:39:07 2012
> @@ -786,7 +786,7 @@
> @__block_holder_tmp_1 = external constant %block1
> define void @test23() {
> entry:
> -  %0 = call i8* @objc_retainBlock(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind
> +  %0 = call i8* @objc_retainBlock(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind, !clang.arc.copy_on_escape !0
>   call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))
>   call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))
>   call void @objc_release(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind
> @@ -801,13 +801,28 @@
> ; CHECK: }
> define void @test23b(i8* %p) {
> entry:
> -  %0 = call i8* @objc_retainBlock(i8* %p) nounwind
> +  %0 = call i8* @objc_retainBlock(i8* %p) nounwind, !clang.arc.copy_on_escape !0
>   call void @callee()
>   call void @use_pointer(i8* %p)
>   call void @objc_release(i8* %p) nounwind
>   ret void
> }
> 
> +; Don't optimize objc_retainBlock, because there's no copy_on_escape metadata.
> +
> +; CHECK: define void @test23c(
> +; CHECK: @objc_retainBlock
> +; CHECK: @objc_release
> +; CHECK: }
> +define void @test23c() {
> +entry:
> +  %0 = call i8* @objc_retainBlock(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind
> +  call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))
> +  call void @bar(i32 ()* bitcast (%block1* @__block_holder_tmp_1 to i32 ()*))
> +  call void @objc_release(i8* bitcast (%block1* @__block_holder_tmp_1 to i8*)) nounwind
> +  ret void
> +}
> +
> ; Any call can decrement a retain count.
> 
> ; CHECK: define void @test24(
> 
> Added: llvm/trunk/test/Transforms/ObjCARC/retain-block.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/retain-block.ll?rev=148076&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/ObjCARC/retain-block.ll (added)
> +++ llvm/trunk/test/Transforms/ObjCARC/retain-block.ll Thu Jan 12 18:39:07 2012
> @@ -0,0 +1,138 @@
> +; RUN: opt -objc-arc -S < %s | FileCheck %s
> +
> +target datalayout = "e-p:64:64:64"
> +
> +!0 = metadata !{}
> +
> +declare i8* @objc_retain(i8*)
> +declare void @callee(i8)
> +declare void @use_pointer(i8*)
> +declare void @objc_release(i8*)
> +declare i8* @objc_retainBlock(i8*)
> +declare i8* @objc_autorelease(i8*)
> +
> +; Basic retainBlock+release elimination.
> +
> +; CHECK: define void @test0(i8* %tmp) {
> +; CHECK-NOT: @objc
> +; CHECK: }
> +define void @test0(i8* %tmp) {
> +entry:
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> +
> +; Same as test0, but there's no copy_on_escape metadata, so there's no
> +; optimization possible.
> +
> +; CHECK: define void @test0_no_metadata(i8* %tmp) {
> +; CHECK: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind
> +; CHECK: tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +; CHECK: }
> +define void @test0_no_metadata(i8* %tmp) {
> +entry:
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> +
> +; Same as test0, but the pointer escapes, so there's no
> +; optimization possible.
> +
> +; CHECK: define void @test0_escape(i8* %tmp, i8** %z) {
> +; CHECK: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +; CHECK: tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +; CHECK: }
> +define void @test0_escape(i8* %tmp, i8** %z) {
> +entry:
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +  store i8* %tmp2, i8** %z
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> +
> +; Same as test0_escape, but there's no intervening call.
> +
> +; CHECK: define void @test0_just_escape(i8* %tmp, i8** %z) {
> +; CHECK: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +; CHECK: tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +; CHECK: }
> +define void @test0_just_escape(i8* %tmp, i8** %z) {
> +entry:
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +  store i8* %tmp2, i8** %z
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> +
> +; Basic nested retainBlock+release elimination.
> +
> +; CHECK: define void @test1(i8* %tmp) {
> +; CHECK-NOT: @objc
> +; CHECK: tail call i8* @objc_retain(i8* %tmp) nounwind
> +; CHECK-NOT: @objc
> +; CHECK: tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0
> +; CHECK-NOT: @objc
> +; CHECK: }
> +define void @test1(i8* %tmp) {
> +entry:
> +  %tmp1 = tail call i8* @objc_retain(i8* %tmp) nounwind
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> +
> +; Same as test1, but there's no copy_on_escape metadata, so there's no
> +; retainBlock+release optimization possible. But we can still eliminate
> +; the outer retain+release.
> +
> +; CHECK: define void @test1_no_metadata(i8* %tmp) {
> +; CHECK-NEXT: entry:
> +; CHECK-NEXT: tail call i8* @objc_retainBlock(i8* %tmp) nounwind
> +; CHECK-NEXT: @use_pointer(i8* %tmp2)
> +; CHECK-NEXT: @use_pointer(i8* %tmp2)
> +; CHECK-NEXT: tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0
> +; CHECK-NOT: @objc
> +; CHECK: }
> +define void @test1_no_metadata(i8* %tmp) {
> +entry:
> +  %tmp1 = tail call i8* @objc_retain(i8* %tmp) nounwind
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> +
> +; Same as test1, but the pointer escapes, so there's no
> +; retainBlock+release optimization possible. But we can still eliminate
> +; the outer retain+release
> +
> +; CHECK: define void @test1_escape(i8* %tmp, i8** %z) {
> +; CHECK-NEXT: entry:
> +; CHECK-NEXT: %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +; CHECK-NEXT: store i8* %tmp2, i8** %z
> +; CHECK-NEXT: @use_pointer(i8* %tmp2)
> +; CHECK-NEXT: @use_pointer(i8* %tmp2)
> +; CHECK-NEXT: tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0
> +; CHECK-NOT: @objc
> +; CHECK: }
> +define void @test1_escape(i8* %tmp, i8** %z) {
> +entry:
> +  %tmp1 = tail call i8* @objc_retain(i8* %tmp) nounwind
> +  %tmp2 = tail call i8* @objc_retainBlock(i8* %tmp) nounwind, !clang.arc.copy_on_escape !0
> +  store i8* %tmp2, i8** %z
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @use_pointer(i8* %tmp2)
> +  tail call void @objc_release(i8* %tmp2) nounwind, !clang.imprecise_release !0
> +  tail call void @objc_release(i8* %tmp) nounwind, !clang.imprecise_release !0
> +  ret void
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120113/a46c096d/attachment.html>


More information about the llvm-commits mailing list