Tighten up the rules for precise lifetime and document

the requirements on the ARC optimizer.

rdar://13407451

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176924 91177308-0d34-0410-b5e6-96231b3b80d8

11 files changed, 353 insertions, 120 deletions

diff --git a/docs/AutomaticReferenceCounting.rst b/docs/AutomaticReferenceCounting.rst
index 65675a44bf..1bbfa0d168 100644
--- a/docs/AutomaticReferenceCounting.rst
+++ b/docs/AutomaticReferenceCounting.rst
@@ -1330,27 +1330,179 @@ This is a new rule of the Objective-C language and applies outside of ARC.
 Optimization
 ============
 
-ARC applies aggressive rules for the optimization of local behavior.  These
-rules are based around a core assumption of :arc-term:`local balancing`: that
-other code will perform retains and releases as necessary (and only as
-necessary) for its own safety, and so the optimizer does not need to consider
-global properties of the retain and release sequence.  For example, if a retain
-and release immediately bracket a call, the optimizer can delete the retain and
-release on the assumption that the called function will not do a constant
-number of unmotivated releases followed by a constant number of "balancing"
-retains, such that the local retain/release pair is the only thing preventing
-the called function from ending up with a dangling reference.
-
-The optimizer assumes that when a new value enters local control, e.g. from a
-load of a non-local object or as the result of a function call, it is
-instaneously valid.  Subsequently, a retain and release of a value are
-necessary on a computation path only if there is a use of that value before the
-release and after any operation which might cause a release of the value
-(including indirectly or non-locally), and only if the value is not
-demonstrably already retained.
-
-The complete optimization rules are quite complicated, but it would still be
-useful to document them here.
+Within this section, the word :arc-term:`function` will be used to
+refer to any structured unit of code, be it a C function, an
+Objective-C method, or a block.
+
+This specification describes ARC as performing specific ``retain`` and
+``release`` operations on retainable object pointers at specific
+points during the execution of a program.  These operations make up a
+non-contiguous subsequence of the computation history of the program.
+The portion of this sequence for a particular retainable object
+pointer for which a specific function execution is directly
+responsible is the :arc-term:`formal local retain history` of the
+object pointer.  The corresponding actual sequence executed is the
+`dynamic local retain history`.
+
+However, under certain circumstances, ARC is permitted to re-order and
+eliminate operations in a manner which may alter the overall
+computation history beyond what is permitted by the general "as if"
+rule of C/C++ and the :ref:`restrictions <_arc.objects.retains>` on
+the implementation of ``retain`` and ``release``.
+
+.. admonition:: Rationale
+
+  Specifically, ARC is sometimes permitted to optimize ``release``
+  operations in ways which might cause an object to be deallocated
+  before it would otherwise be.  Without this, it would be almost
+  impossible to eliminate any ``retain``/``release`` pairs.  For
+  example, consider the following code:
+
+  .. code-block:: objc
+    id x = _ivar;
+    [x foo];
+
+  If we were not permitted in any event to shorten the lifetime of the
+  object in ``x``, then we would not be able to eliminate this retain
+  and release unless we could prove that the message send could not
+  modify ``_ivar`` (or deallocate ``self``).  Since message sends are
+  opaque to the optimizer, this is not possible, and so ARC's hands
+  would be almost completely tied.
+
+ARC makes no guarantees about the execution of a computation history
+which contains undefined behavior.  In particular, ARC makes no
+guarantees in the presence of race conditions.
+
+ARC may assume that any retainable object pointers it receives or
+generates are instantaneously valid from that point until a point
+which, by the concurrency model of the host language, happens-after
+the generation of the pointer and happens-before a release of that
+object (possibly via an aliasing pointer or indirectly due to
+destruction of a different object).
+
+.. admonition:: Rationale
+
+  There is very little point in trying to guarantee correctness in the
+  presence of race conditions.  ARC does not have a stack-scanning
+  garbage collector, and guaranteeing the atomicity of every load and
+  store operation would be prohibitive and preclude a vast amount of
+  optimization.
+
+ARC may assume that non-ARC code engages in sensible balancing
+behavior and does not rely on exact or minimum retain count values
+except as guaranteed by ``__strong`` object invariants or +1 transfer
+conventions.  For example, if an object is provably double-retained
+and double-released, ARC may eliminate the inner retain and release;
+it does not need to guard against code which performs an unbalanced
+release followed by a "balancing" retain.
+
+.. _arc.optimization.liveness:
+
+Object liveness
+---------------
+
+ARC may not allow a retainable object ``X`` to be deallocated at a
+time ``T`` in a computation history if:
+
+* ``X`` is the value stored in a ``__strong`` object ``S`` with
+  :ref:`precise lifetime semantics <arc.optimization.precise>`, or
+
+* ``X`` is the value stored in a ``__strong`` object ``S`` with
+  imprecise lifetime semantics and, at some point after ``T`` but
+  before the next store to ``S``, the computation history features a
+  load from ``S`` and in some way depends on the value loaded, or
+
+* ``X`` is a value described as being released at the end of the
+  current full-expression and, at some point after ``T`` but before
+  the end of the full-expression, the computation history depends
+  on that value.
+
+.. admonition:: Rationale
+
+  The intent of the second rule is to say that objects held in normal
+  ``__strong`` local variables may be released as soon as the value in
+  the variable is no longer being used: either the variable stops
+  being used completely or a new value is stored in the variable.
+
+  The intent of the third rule is to say that return values may be
+  released after they've been used.
+
+A computation history depends on a pointer value ``P`` if it:
+
+* performs a pointer comparison with ``P``,
+* loads from ``P``,
+* stores to ``P``,
+* depends on a pointer value ``Q`` derived via pointer arithmetic
+  from ``P`` (including an instance-variable or field access), or
+* depends on a pointer value ``Q`` loaded from ``P``.
+
+Dependency applies only to values derived directly or indirectly from
+a particular expression result and does not occur merely because a
+separate pointer value dynamically aliases ``P``.  Furthermore, this
+dependency is not carried by values that are stored to objects.
+
+.. admonition:: Rationale
+
+  The restrictions on dependency are intended to make this analysis
+  feasible by an optimizer with only incomplete information about a
+  program.  Essentially, dependence is carried to "obvious" uses of a
+  pointer.  Merely passing a pointer argument to a function does not
+  itself cause dependence, but since generally the optimizer will not
+  be able to prove that the function doesn't depend on that parameter,
+  it will be forced to conservatively assume it does.
+  
+  Dependency propagates to values loaded from a pointer because those
+  values might be invalidated by deallocating the object.  For
+  example, given the code ``__strong id x = p->ivar;``, ARC must not
+  move the release of ``p`` to between the load of ``p->ivar`` and the
+  retain of that value for storing into ``x``.
+
+  Dependency does not propagate through stores of dependent pointer
+  values because doing so would allow dependency to outlive the
+  full-expression which produced the original value.  For example, the
+  address of an instance variable could be written to some global
+  location and then freely accessed during the lifetime of the local,
+  or a function could return an inner pointer of an object and store
+  it to a local.  These cases would be potentially impossible to
+  reason about and so would basically prevent any optimizations based
+  on imprecise lifetime.  There are also uncommon enough to make it
+  reasonable to require the precise-lifetime annotation if someone
+  really wants to rely on them.
+
+  Dependency does propagate through return values of pointer type.
+  The compelling source of need for this rule is a property accessor
+  which returns an un-autoreleased result; the calling function must
+  have the chance to operate on the value, e.g. to retain it, before
+  ARC releases the original pointer.  Note again, however, that
+  dependence does not survive a store, so ARC does not guarantee the
+  continued validity of the return value past the end of the
+  full-expression.
+
+.. _arc.optimization.object_lifetime:
+
+No object lifetime extension
+----------------------------
+
+If, in the formal computation history of the program, an object ``X``
+has been deallocated by the time of an observable side-effect, then
+ARC must cause ``X`` to be deallocated by no later than the occurrence
+of that side-effect, except as influenced by the re-ordering of the
+destruction of objects.
+
+.. admonition:: Rationale
+
+  This rule is intended to prohibit ARC from observably extending the
+  lifetime of a retainable object, other than as specified in this
+  document.  Together with the rule limiting the transformation of
+  releases, this rule requires ARC to eliminate retains and release
+  only in pairs.
+
+  ARC's power to reorder the destruction of objects is critical to its
+  ability to do any optimization, for essentially the same reason that
+  it must retain the power to decrease the lifetime of an object.
+  Unfortunately, while it's generally poor style for the destruction
+  of objects to have arbitrary side-effects, it's certainly possible.
+  Hence the caveat.
 
 .. _arc.optimization.precise:
 
diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index b9f466117c..c521bf9472 100644
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -1547,7 +1547,7 @@ CodeGenFunction::GenerateDestroyHelperFunction(const CGBlockInfo &blockInfo) {
 
     // Destroy strong objects with a call if requested.
     } else if (useARCStrongDestroy) {
-      EmitARCDestroyStrong(srcField, /*precise*/ false);
+      EmitARCDestroyStrong(srcField, ARCImpreciseLifetime);
 
     // Otherwise we call _Block_object_dispose.  It wouldn't be too
     // hard to just emit this as a cleanup if we wanted to make sure
@@ -1656,7 +1656,7 @@ public:
   }
 
   void emitDispose(CodeGenFunction &CGF, llvm::Value *field) {
-    CGF.EmitARCDestroyStrong(field, /*precise*/ false);
+    CGF.EmitARCDestroyStrong(field, ARCImpreciseLifetime);
   }
 
   void profileImpl(llvm::FoldingSetNodeID &id) const {
@@ -1686,7 +1686,7 @@ public:
   }
 
   void emitDispose(CodeGenFunction &CGF, llvm::Value *field) {
-    CGF.EmitARCDestroyStrong(field, /*precise*/ false);
+    CGF.EmitARCDestroyStrong(field, ARCImpreciseLifetime);
   }
 
   void profileImpl(llvm::FoldingSetNodeID &id) const {
diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp
index 0e001301ae..f9f48ae9ca 100644
--- a/lib/CodeGen/CGDecl.cpp
+++ b/lib/CodeGen/CGDecl.cpp
@@ -627,7 +627,7 @@ void CodeGenFunction::EmitScalarInit(const Expr *init,
   if (accessedByInit && lifetime == Qualifiers::OCL_Strong) {
     llvm::Value *oldValue = EmitLoadOfScalar(lvalue);
     EmitStoreOfScalar(value, lvalue, /* isInitialization */ true);
-    EmitARCRelease(oldValue, /*precise*/ false);
+    EmitARCRelease(oldValue, ARCImpreciseLifetime);
     return;
   }
 
@@ -1490,12 +1490,15 @@ namespace {
   /// ns_consumed argument when we can't reasonably do that just by
   /// not doing the initial retain for a __block argument.
   struct ConsumeARCParameter : EHScopeStack::Cleanup {
-    ConsumeARCParameter(llvm::Value *param) : Param(param) {}
+    ConsumeARCParameter(llvm::Value *param,
+                        ARCPreciseLifetime_t precise)
+      : Param(param), Precise(precise) {}
 
     llvm::Value *Param;
+    ARCPreciseLifetime_t Precise;
 
     void Emit(CodeGenFunction &CGF, Flags flags) {
-      CGF.EmitARCRelease(Param, /*precise*/ false);
+      CGF.EmitARCRelease(Param, Precise);
     }
   };
 }
@@ -1585,8 +1588,12 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg,
         }
       } else {
         // Push the cleanup for a consumed parameter.
-        if (isConsumed)
-          EHStack.pushCleanup<ConsumeARCParameter>(getARCCleanupKind(), Arg);
+        if (isConsumed) {
+          ARCPreciseLifetime_t precise = (D.hasAttr<ObjCPreciseLifetimeAttr>()
+                                ? ARCPreciseLifetime : ARCImpreciseLifetime);
+          EHStack.pushCleanup<ConsumeARCParameter>(getARCCleanupKind(), Arg,
+                                                   precise);
+        }
 
         if (lt == Qualifiers::OCL_Weak) {
           EmitARCInitWeak(DeclPtr, Arg);
diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 5e6342ae38..e551343119 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1800,10 +1800,6 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
 
     bool isBlockVariable = VD->hasAttr<BlocksAttr>();
 
-    bool NonGCable = VD->hasLocalStorage() &&
-                     !VD->getType()->isReferenceType() &&
-                     !isBlockVariable;
-
     llvm::Value *V = LocalDeclMap.lookup(VD);
     if (!V && VD->isStaticLocal()) 
       V = CGM.getStaticLocalDeclAddress(VD);
@@ -1837,10 +1833,20 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
       LV = MakeAddrLValue(V, T, Alignment);
     }
 
+    bool isLocalStorage = VD->hasLocalStorage();
+
+    bool NonGCable = isLocalStorage &&
+                     !VD->getType()->isReferenceType() &&
+                     !isBlockVariable;
     if (NonGCable) {
       LV.getQuals().removeObjCGCAttr();
       LV.setNonGC(true);
     }
+
+    bool isImpreciseLifetime =
+      (isLocalStorage && !VD->hasAttr<ObjCPreciseLifetimeAttr>());
+    if (isImpreciseLifetime)
+      LV.setARCPreciseLifetime(ARCImpreciseLifetime);
     setObjCGCLValueClass(getContext(), E, LV);
     return LV;
   }
@@ -2911,7 +2917,7 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
       case Qualifiers::OCL_Strong:
         EmitARCRelease(Builder.CreateLoad(BaseValue, 
                           PseudoDtor->getDestroyedType().isVolatileQualified()),
-                       /*precise*/ true);
+                       ARCPreciseLifetime);
         break;
 
       case Qualifiers::OCL_Weak:
diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index 13ae8bb01a..83c8ace98c 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -1451,7 +1451,7 @@ static void EmitObjectDelete(CodeGenFunction &CGF,
       llvm::Value *PtrValue = CGF.Builder.CreateLoad(Ptr, 
                                              ElementType.isVolatileQualified());
         
-      CGF.EmitARCRelease(PtrValue, /*precise*/ true);
+      CGF.EmitARCRelease(PtrValue, ARCPreciseLifetime);
       break;
     }
         
diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp
index ad7d62951a..f53f2a97c6 100644
--- a/lib/CodeGen/CGObjC.cpp
+++ b/lib/CodeGen/CGObjC.cpp
@@ -1686,7 +1686,8 @@ namespace {
     llvm::Value *object;
 
     void Emit(CodeGenFunction &CGF, Flags flags) {
-      CGF.EmitARCRelease(object, /*precise*/ true);
+      // Releases at the end of the full-expression are imprecise.
+      CGF.EmitARCRelease(object, ARCImpreciseLifetime);
     }
   };
 }
@@ -1940,7 +1941,8 @@ CodeGenFunction::EmitARCRetainAutoreleasedReturnValue(llvm::Value *value) {
 
 /// Release the given object.
 ///   call void \@objc_release(i8* %value)
-void CodeGenFunction::EmitARCRelease(llvm::Value *value, bool precise) {
+void CodeGenFunction::EmitARCRelease(llvm::Value *value,
+                                     ARCPreciseLifetime_t precise) {
   if (isa<llvm::ConstantPointerNull>(value)) return;
 
   llvm::Constant *&fn = CGM.getARCEntrypoints().objc_release;
@@ -1956,7 +1958,7 @@ void CodeGenFunction::EmitARCRelease(llvm::Value *value, bool precise) {
   // Call objc_release.
   llvm::CallInst *call = EmitNounwindRuntimeCall(fn, value);
 
-  if (!precise) {
+  if (precise == ARCImpreciseLifetime) {
     SmallVector<llvm::Value*,1> args;
     call->setMetadata("clang.imprecise_release",
                       llvm::MDNode::get(Builder.getContext(), args));
@@ -1972,7 +1974,8 @@ void CodeGenFunction::EmitARCRelease(llvm::Value *value, bool precise) {
 /// At -O1 and above, just load and call objc_release.
 ///
 ///   call void \@objc_storeStrong(i8** %addr, i8* null)
-void CodeGenFunction::EmitARCDestroyStrong(llvm::Value *addr, bool precise) {
+void CodeGenFunction::EmitARCDestroyStrong(llvm::Value *addr,
+                                           ARCPreciseLifetime_t precise) {
   if (CGM.getCodeGenOpts().OptimizationLevel == 0) {
     llvm::PointerType *addrTy = cast<llvm::PointerType>(addr->getType());
     llvm::Value *null = llvm::ConstantPointerNull::get(
@@ -2042,7 +2045,7 @@ llvm::Value *CodeGenFunction::EmitARCStoreStrong(LValue dst,
   EmitStoreOfScalar(newValue, dst);
 
   // Finally, release the old value.
-  EmitARCRelease(oldValue, /*precise*/ false);
+  EmitARCRelease(oldValue, dst.isARCPreciseLifetime());
 
   return newValue;
 }
@@ -2254,13 +2257,13 @@ void CodeGenFunction::EmitObjCMRRAutoreleasePoolPop(llvm::Value *Arg) {
 void CodeGenFunction::destroyARCStrongPrecise(CodeGenFunction &CGF,
                                               llvm::Value *addr,
                                               QualType type) {
-  CGF.EmitARCDestroyStrong(addr, /*precise*/ true);
+  CGF.EmitARCDestroyStrong(addr, ARCPreciseLifetime);
 }
 
 void CodeGenFunction::destroyARCStrongImprecise(CodeGenFunction &CGF,
                                                 llvm::Value *addr,
                                                 QualType type) {
-  CGF.EmitARCDestroyStrong(addr, /*precise*/ false);
+  CGF.EmitARCDestroyStrong(addr, ARCImpreciseLifetime);
 }
 
 void CodeGenFunction::destroyARCWeak(CodeGenFunction &CGF,
@@ -2737,7 +2740,7 @@ CodeGenFunction::EmitARCStoreStrong(const BinaryOperator *e,
     llvm::Value *oldValue =
       EmitLoadOfScalar(lvalue);
     EmitStoreOfScalar(value, lvalue);
-    EmitARCRelease(oldValue, /*precise*/ false);
+    EmitARCRelease(oldValue, lvalue.isARCPreciseLifetime());
   } else {
     value = EmitARCStoreStrong(lvalue, value, ignored);
   }
diff --git a/lib/CodeGen/CGObjCMac.cpp b/lib/CodeGen/CGObjCMac.cpp
index 975f9ac4af..019c32ea3d 100644
--- a/lib/CodeGen/CGObjCMac.cpp
+++ b/lib/CodeGen/CGObjCMac.cpp
@@ -1617,7 +1617,7 @@ struct NullReturnState {
           RValue RV = I->RV;
           assert(RV.isScalar() && 
                  "NullReturnState::complete - arg not on object");
-          CGF.EmitARCRelease(RV.getScalarVal(), true);
+          CGF.EmitARCRelease(RV.getScalarVal(), ARCImpreciseLifetime);
         }
       }
     }
diff --git a/lib/CodeGen/CGValue.h b/lib/CodeGen/CGValue.h
index 6b0c271a08..b7591a76fb 100644
--- a/lib/CodeGen/CGValue.h
+++ b/lib/CodeGen/CGValue.h
@@ -97,6 +97,10 @@ public:
   }
 };
 
+/// Does an ARC strong l-value have precise lifetime?
+enum ARCPreciseLifetime_t {
+  ARCImpreciseLifetime, ARCPreciseLifetime,
+};
 
 /// LValue - This represents an lvalue references.  Because C/C++ allow
 /// bitfields, this is not a simple LLVM pointer, it may be a pointer plus a
@@ -147,6 +151,10 @@ class LValue {
   // Lvalue is a thread local reference
   bool ThreadLocalRef : 1;
 
+  // Lvalue has ARC imprecise lifetime.  We store this inverted to try
+  // to make the default bitfield pattern all-zeroes.
+  bool ImpreciseLifetime : 1;
+
   Expr *BaseIvarExp;
 
   /// TBAAInfo - TBAA information to attach to dereferences of this LValue.
@@ -164,6 +172,7 @@ private:
 
     // Initialize Objective-C flags.
     this->Ivar = this->ObjIsArray = this->NonGC = this->GlobalObjCRef = false;
+    this->ImpreciseLifetime = false;
     this->ThreadLocalRef = false;
     this->BaseIvarExp = 0;
     this->TBAAInfo = TBAAInfo;
@@ -202,6 +211,13 @@ public:
   bool isThreadLocalRef() const { return ThreadLocalRef; }
   void setThreadLocalRef(bool Value) { ThreadLocalRef = Value;}
 
+  ARCPreciseLifetime_t isARCPreciseLifetime() const {
+    return ARCPreciseLifetime_t(!ImpreciseLifetime);
+  }
+  void setARCPreciseLifetime(ARCPreciseLifetime_t value) {
+    ImpreciseLifetime = (value == ARCImpreciseLifetime);
+  }
+
   bool isObjCWeak() const {
     return Quals.getObjCGCAttr() == Qualifiers::Weak;
   }
diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 0155f033e6..55c21e80ee 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -2446,14 +2446,14 @@ public:
   llvm::Value *EmitARCRetainAutorelease(QualType type, llvm::Value *value);
   llvm::Value *EmitARCRetainAutoreleaseNonBlock(llvm::Value *value);
   llvm::Value *EmitARCStoreStrong(LValue lvalue, llvm::Value *value,
-                                  bool ignored);
+                                  bool resultIgnored);
   llvm::Value *EmitARCStoreStrongCall(llvm::Value *addr, llvm::Value *value,
-                                      bool ignored);
+                                      bool resultIgnored);
   llvm::Value *EmitARCRetain(QualType type, llvm::Value *value);
   llvm::Value *EmitARCRetainNonBlock(llvm::Value *value);
   llvm::Value *EmitARCRetainBlock(llvm::Value *value, bool mandatory);
-  void EmitARCDestroyStrong(llvm::Value *addr, bool precise);
-  void EmitARCRelease(llvm::Value *value, bool precise);
+  void EmitARCDestroyStrong(llvm::Value *addr, ARCPreciseLifetime_t precise);
+  void EmitARCRelease(llvm::Value *value, ARCPreciseLifetime_t precise);
   llvm::Value *EmitARCAutorelease(llvm::Value *value);
   llvm::Value *EmitARCAutoreleaseReturnValue(llvm::Value *value);
   llvm::Value *EmitARCRetainAutoreleaseReturnValue(llvm::Value *value);
diff --git a/test/CodeGenObjC/arc-precise-lifetime.m b/test/CodeGenObjC/arc-precise-lifetime.m
new file mode 100644
index 0000000000..595a4f9fdf
--- /dev/null
+++ b/test/CodeGenObjC/arc-precise-lifetime.m
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -O2 -disable-llvm-optzns -o - %s | FileCheck %s
+
+#define PRECISE_LIFETIME __attribute__((objc_precise_lifetime))
+
+id test0_helper(void) __attribute__((ns_returns_retained));
+void test0() {
+  PRECISE_LIFETIME id x = test0_helper();
+  x = 0;
+  // CHECK:      [[X:%.*]] = alloca i8*
+  // CHECK-NEXT: [[CALL:%.*]] = call i8* @test0_helper()
+  // CHECK-NEXT: store i8* [[CALL]], i8** [[X]]
+
+  // CHECK-NEXT: [[T1:%.*]] = load i8** [[X]]
+  // CHECK-NEXT: store i8* null, i8** [[X]]
+  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW:#[0-9]+]]
+  // CHECK-NOT:  clang.imprecise_release
+
+  // CHECK-NEXT: [[T1:%.*]] = load i8** [[X]]
+  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW:#[0-9]+]]
+  // CHECK-NOT:  clang.imprecise_release
+
+  // CHECK-NEXT: ret void
+}
+
+// rdar://problem/9821110
+@interface Test1
+- (char*) interior __attribute__((objc_returns_inner_pointer));
+// Should we allow this on properties?
+@end
+extern Test1 *test1_helper(void);
+
+// CHECK: define void @test1a()
+void test1a(void) {
+  // CHECK:      [[T0:%.*]] = call [[TEST1:%.*]]* @test1_helper()
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST1]]* [[T0]] to i8*
+  // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T1]])
+  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[TEST1]]*
+  // CHECK-NEXT: store [[TEST1]]* [[T3]]
+  // CHECK-NEXT: [[T0:%.*]] = load [[TEST1]]**
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST1]]* [[T0]] to i8*
+  // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainAutorelease(i8* [[T1]])
+  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[TEST1]]*
+  // CHECK-NEXT: [[T4:%.*]] = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_
+  // CHECK-NEXT: [[T5:%.*]] = bitcast [[TEST1]]* [[T3]] to i8*
+  // CHECK-NEXT: [[T6:%.*]] = call i8* bitcast
+  // CHECK-NEXT: store i8* [[T6]], i8**
+  // CHECK-NEXT: [[T0:%.*]] = load [[TEST1]]**
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST1]]* [[T0]] to i8*
+  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]], !clang.imprecise_release
+  // CHECK-NEXT: ret void
+  Test1 *ptr = test1_helper();
+  char *c = [(ptr) interior];
+}
+
+// CHECK: define void @test1b()
+void test1b(void) {
+  // CHECK:      [[T0:%.*]] = call [[TEST1:%.*]]* @test1_helper()
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST1]]* [[T0]] to i8*
+  // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T1]])
+  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[TEST1]]*
+  // CHECK-NEXT: store [[TEST1]]* [[T3]]
+  // CHECK-NEXT: [[T0:%.*]] = load [[TEST1]]**
+  // CHECK-NEXT: [[T1:%.*]] = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_
+  // CHECK-NEXT: [[T2:%.*]] = bitcast [[TEST1]]* [[T0]] to i8*
+  // CHECK-NEXT: [[T3:%.*]] = call i8* bitcast
+  // CHECK-NEXT: store i8* [[T3]], i8**
+  // CHECK-NEXT: [[T0:%.*]] = load [[TEST1]]**
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST1]]* [[T0]] to i8*
+  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]]
+  // CHECK-NOT:  clang.imprecise_release
+  // CHECK-NEXT: ret void
+  __attribute__((objc_precise_lifetime)) Test1 *ptr = test1_helper();
+  char *c = [ptr interior];
+}
+
+@interface Test2 {
+@public
+  id ivar;
+}
+@end
+// CHECK:      define void @test2(
+void test2(Test2 *x) {
+  x->ivar = 0;
+  // CHECK:      [[X:%.*]] = alloca [[TEST2:%.*]]*
+  // CHECK-NEXT: [[T0:%.*]] = bitcast [[TEST2]]* {{%.*}} to i8*
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retain(i8* [[T0]]) [[NUW]]
+  // CHECK-NEXT: [[T2:%.*]] = bitcast i8* [[T1]] to [[TEST2]]*
+  // CHECK-NEXT: store [[TEST2]]* [[T2]], [[TEST2]]** [[X]],
+
+  // CHECK-NEXT: [[T0:%.*]] = load [[TEST2]]** [[X]],
+  // CHECK-NEXT: [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test2.ivar"
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST2]]* [[T0]] to i8*
+  // CHECK-NEXT: [[T2:%.*]] = getelementptr inbounds i8* [[T1]], i64 [[OFFSET]]
+  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to i8**
+  // CHECK-NEXT: [[T4:%.*]] = load i8** [[T3]],
+  // CHECK-NEXT: store i8* null, i8** [[T3]],
+  // CHECK-NEXT: call void @objc_release(i8* [[T4]]) [[NUW]]
+  // CHECK-NOT:  imprecise
+
+  // CHECK-NEXT: [[T0:%.*]] = load [[TEST2]]** [[X]]
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST2]]* [[T0]] to i8*
+  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]], !clang.imprecise_release
+
+  // CHECK-NEXT: ret void
+}
+
+// CHECK:      define void @test3(i8*
+void test3(PRECISE_LIFETIME id x) {
+  // CHECK:      [[X:%.*]] = alloca i8*,
+  // CHECK-NEXT: [[T0:%.*]] = call i8* @objc_retain(i8* {{%.*}}) [[NUW]]
+  // CHECK-NEXT: store i8* [[T0]], i8** [[X]],
+
+  // CHECK-NEXT: [[T0:%.*]] = load i8** [[X]]
+  // CHECK-NEXT: call void @objc_release(i8* [[T0]]) [[NUW]]
+  // CHECK-NOT:  imprecise_release
+
+  // CHECK-NEXT: ret void  
+}
+
+// CHECK: attributes [[NUW]] = { nounwind }
diff --git a/test/CodeGenObjC/arc.m b/test/CodeGenObjC/arc.m
index 48f012a42b..0ef3d89510 100644
--- a/test/CodeGenObjC/arc.m
+++ b/test/CodeGenObjC/arc.m
@@ -273,27 +273,7 @@ void test8() {
   // CHECK:      [[X:%.*]] = alloca i8*
   // CHECK-NEXT: [[T0:%.*]] = call i8* @test8_helper()
   // CHECK-NEXT: store i8* [[T0]], i8** [[X]]
-  // CHECK-NEXT: call void @objc_release(i8* [[T0]]) [[NUW]]
-  // CHECK-NOT:  imprecise_release
-  // CHECK-NEXT: ret void
-}
-
-id test9_helper(void) __attribute__((ns_returns_retained));
-void test9() {
-  id x __attribute__((objc_precise_lifetime)) = test9_helper();
-  x = 0;
-  // CHECK:      [[X:%.*]] = alloca i8*
-  // CHECK-NEXT: [[CALL:%.*]] = call i8* @test9_helper()
-  // CHECK-NEXT: store i8* [[CALL]], i8** [[X]]
-
-  // CHECK-NEXT: [[T1:%.*]] = load i8** [[X]]
-  // CHECK-NEXT: store i8* null, i8** [[X]]
-  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]], !clang.imprecise_release
-
-  // CHECK-NEXT: [[T1:%.*]] = load i8** [[X]]
-  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]]
-  // CHECK-NOT:  clang.imprecise_release
-
+  // CHECK-NEXT: call void @objc_release(i8* [[T0]]) [[NUW]], !clang.imprecise_release
   // CHECK-NEXT: ret void
 }
 
@@ -1239,57 +1219,6 @@ void test56_test(void) {
 // CHECK-NEXT: [[T5:%.*]] = load i8** [[T4]]
 // CHECK-NEXT: ret i8* [[T5]]
 
-// rdar://problem/9821110
-@interface Test58
-- (char*) interior __attribute__((objc_returns_inner_pointer));
-// Should we allow this on properties?
-@end
-extern Test58 *test58_helper(void);
-
-// CHECK: define void @test58a()
-void test58a(void) {
-  // CHECK:      [[T0:%.*]] = call [[TEST58:%.*]]* @test58_helper()
-  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST58]]* [[T0]] to i8*
-  // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T1]])
-  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[TEST58]]*
-  // CHECK-NEXT: store [[TEST58]]* [[T3]]
-  // CHECK-NEXT: [[T0:%.*]] = load [[TEST58]]**
-  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST58]]* [[T0]] to i8*
-  // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainAutorelease(i8* [[T1]])
-  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[TEST58]]*
-  // CHECK-NEXT: [[T4:%.*]] = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_
-  // CHECK-NEXT: [[T5:%.*]] = bitcast [[TEST58]]* [[T3]] to i8*
-  // CHECK-NEXT: [[T6:%.*]] = call i8* bitcast
-  // CHECK-NEXT: store i8* [[T6]], i8**
-  // CHECK-NEXT: [[T0:%.*]] = load [[TEST58]]**
-  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST58]]* [[T0]] to i8*
-  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]], !clang.imprecise_release
-  // CHECK-NEXT: ret void
-  Test58 *ptr = test58_helper();
-  char *c = [(ptr) interior];
-}
-
-// CHECK: define void @test58b()
-void test58b(void) {
-  // CHECK:      [[T0:%.*]] = call [[TEST58:%.*]]* @test58_helper()
-  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST58]]* [[T0]] to i8*
-  // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T1]])
-  // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[TEST58]]*
-  // CHECK-NEXT: store [[TEST58]]* [[T3]]
-  // CHECK-NEXT: [[T0:%.*]] = load [[TEST58]]**
-  // CHECK-NEXT: [[T1:%.*]] = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_
-  // CHECK-NEXT: [[T2:%.*]] = bitcast [[TEST58]]* [[T0]] to i8*
-  // CHECK-NEXT: [[T3:%.*]] = call i8* bitcast
-  // CHECK-NEXT: store i8* [[T3]], i8**
-  // CHECK-NEXT: [[T0:%.*]] = load [[TEST58]]**
-  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST58]]* [[T0]] to i8*
-  // CHECK-NEXT: call void @objc_release(i8* [[T1]]) [[NUW]]
-  // CHECK-NOT:  clang.imprecise_release
-  // CHECK-NEXT: ret void
-  __attribute__((objc_precise_lifetime)) Test58 *ptr = test58_helper();
-  char *c = [ptr interior];
-}
-
 // rdar://problem/9842343
 void test59(void) {
   extern id test59_getlock(void);