At -O0, prefer objc_storeStrong with a null new value to the

combination of a load+objc_release;  this is generally better
for tools that try to track why values are retained and
released.  Also use objc_storeStrong when copying a block
(again, only at -O0), which requires us to do a preliminary
store of null in order to compensate for objc_storeStrong's
assign semantics.

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

3 files changed, 89 insertions, 28 deletions

diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 33f4bf1670..b178f5eac1 100644
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -1241,7 +1241,8 @@ CodeGenFunction::GenerateCopyHelperFunction(const CGBlockInfo &blockInfo) {
     const Expr *copyExpr = ci->getCopyExpr();
     BlockFieldFlags flags;
 
-    bool isARCWeakCapture = false;
+    bool useARCWeakCopy = false;
+    bool useARCStrongCopy = false;
 
     if (copyExpr) {
       assert(!ci->isByRef());
@@ -1254,21 +1255,35 @@ CodeGenFunction::GenerateCopyHelperFunction(const CGBlockInfo &blockInfo) {
 
     } else if (type->isObjCRetainableType()) {
       flags = BLOCK_FIELD_IS_OBJECT;
-      if (type->isBlockPointerType())
+      bool isBlockPointer = type->isBlockPointerType();
+      if (isBlockPointer)
         flags = BLOCK_FIELD_IS_BLOCK;
 
       // Special rules for ARC captures:
       if (getLangOpts().ObjCAutoRefCount) {
         Qualifiers qs = type.getQualifiers();
 
-        // Don't generate special copy logic for a captured object
-        // unless it's __strong or __weak.
-        if (!qs.hasStrongOrWeakObjCLifetime())
+        // We need to register __weak direct captures with the runtime.
+        if (qs.getObjCLifetime() == Qualifiers::OCL_Weak) {
+          useARCWeakCopy = true;
+
+        // We need to retain the copied value for __strong direct captures.
+        } else if (qs.getObjCLifetime() == Qualifiers::OCL_Strong) {
+          // If it's a block pointer, we have to copy the block and
+          // assign that to the destination pointer, so we might as
+          // well use _Block_object_assign.  Otherwise we can avoid that.
+          if (!isBlockPointer)
+            useARCStrongCopy = true;
+
+        // Otherwise the memcpy is fine.
+        } else {
           continue;
+        }
 
-        // Support __weak direct captures.
-        if (qs.getObjCLifetime() == Qualifiers::OCL_Weak)
-          isARCWeakCapture = true;
+      // Non-ARC captures of retainable pointers are strong and
+      // therefore require a call to _Block_object_assign.
+      } else {
+        // fall through
       }
     } else {
       continue;
@@ -1281,14 +1296,36 @@ CodeGenFunction::GenerateCopyHelperFunction(const CGBlockInfo &blockInfo) {
     // If there's an explicit copy expression, we do that.
     if (copyExpr) {
       EmitSynthesizedCXXCopyCtor(dstField, srcField, copyExpr);
-    } else if (isARCWeakCapture) {
+    } else if (useARCWeakCopy) {
       EmitARCCopyWeak(dstField, srcField);
     } else {
       llvm::Value *srcValue = Builder.CreateLoad(srcField, "blockcopy.src");
-      srcValue = Builder.CreateBitCast(srcValue, VoidPtrTy);
-      llvm::Value *dstAddr = Builder.CreateBitCast(dstField, VoidPtrTy);
-      Builder.CreateCall3(CGM.getBlockObjectAssign(), dstAddr, srcValue,
-                          llvm::ConstantInt::get(Int32Ty, flags.getBitMask()));
+      if (useARCStrongCopy) {
+        // At -O0, store null into the destination field (so that the
+        // storeStrong doesn't over-release) and then call storeStrong.
+        // This is a workaround to not having an initStrong call.
+        if (CGM.getCodeGenOpts().OptimizationLevel == 0) {
+          llvm::PointerType *ty = cast<llvm::PointerType>(srcValue->getType());
+          llvm::Value *null = llvm::ConstantPointerNull::get(ty);
+          Builder.CreateStore(null, dstField);
+          EmitARCStoreStrongCall(dstField, srcValue, true);
+
+        // With optimization enabled, take advantage of the fact that
+        // the blocks runtime guarantees a memcpy of the block data, and
+        // just emit a retain of the src field.
+        } else {
+          EmitARCRetainNonBlock(srcValue);
+
+          // We don't need this anymore, so kill it.  It's not quite
+          // worth the annoyance to avoid creating it in the first place.
+          cast<llvm::Instruction>(dstField)->eraseFromParent();
+        }
+      } else {
+        srcValue = Builder.CreateBitCast(srcValue, VoidPtrTy);
+        llvm::Value *dstAddr = Builder.CreateBitCast(dstField, VoidPtrTy);
+        Builder.CreateCall3(CGM.getBlockObjectAssign(), dstAddr, srcValue,
+                            llvm::ConstantInt::get(Int32Ty, flags.getBitMask()));
+      }
     }
   }
 
@@ -1353,7 +1390,8 @@ CodeGenFunction::GenerateDestroyHelperFunction(const CGBlockInfo &blockInfo) {
     BlockFieldFlags flags;
     const CXXDestructorDecl *dtor = 0;
 
-    bool isARCWeakCapture = false;
+    bool useARCWeakDestroy = false;
+    bool useARCStrongDestroy = false;
 
     if (ci->isByRef()) {
       flags = BLOCK_FIELD_IS_BYREF;
@@ -1379,7 +1417,11 @@ CodeGenFunction::GenerateDestroyHelperFunction(const CGBlockInfo &blockInfo) {
 
         // Support __weak direct captures.
         if (qs.getObjCLifetime() == Qualifiers::OCL_Weak)
-          isARCWeakCapture = true;
+          useARCWeakDestroy = true;
+
+        // Tools really want us to use objc_storeStrong here.
+        else
+          useARCStrongDestroy = true;
       }
     } else {
       continue;
@@ -1393,9 +1435,13 @@ CodeGenFunction::GenerateDestroyHelperFunction(const CGBlockInfo &blockInfo) {
       PushDestructorCleanup(dtor, srcField);
 
     // If this is a __weak capture, emit the release directly.
-    } else if (isARCWeakCapture) {
+    } else if (useARCWeakDestroy) {
       EmitARCDestroyWeak(srcField);
 
+    // Destroy strong objects with a call if requested.
+    } else if (useARCStrongDestroy) {
+      EmitARCDestroyStrong(srcField, /*precise*/ false);
+
     // 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
     // that things were done in reverse.
@@ -1494,10 +1540,7 @@ public:
   }
 
   void emitDispose(CodeGenFunction &CGF, llvm::Value *field) {
-    llvm::LoadInst *value = CGF.Builder.CreateLoad(field);
-    value->setAlignment(Alignment.getQuantity());
-
-    CGF.EmitARCRelease(value, /*precise*/ false);
+    CGF.EmitARCDestroyStrong(field, /*precise*/ false);
   }
 
   void profileImpl(llvm::FoldingSetNodeID &id) const {
@@ -1527,10 +1570,7 @@ public:
   }
 
   void emitDispose(CodeGenFunction &CGF, llvm::Value *field) {
-    llvm::LoadInst *value = CGF.Builder.CreateLoad(field);
-    value->setAlignment(Alignment.getQuantity());
-
-    CGF.EmitARCRelease(value, /*precise*/ false);
+    CGF.EmitARCDestroyStrong(field, /*precise*/ false);
   }
 
   void profileImpl(llvm::FoldingSetNodeID &id) const {
diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp
index 0fa70e7d3d..a47c56f67c 100644
--- a/lib/CodeGen/CGObjC.cpp
+++ b/lib/CodeGen/CGObjC.cpp
@@ -1949,6 +1949,28 @@ void CodeGenFunction::EmitARCRelease(llvm::Value *value, bool precise) {
   }
 }
 
+/// Destroy a __strong variable.
+///
+/// At -O0, emit a call to store 'null' into the address;
+/// instrumenting tools prefer this because the address is exposed,
+/// but it's relatively cumbersome to optimize.
+///
+/// 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) {
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0) {
+    llvm::PointerType *addrTy = cast<llvm::PointerType>(addr->getType());
+    llvm::Value *null = llvm::ConstantPointerNull::get(
+                          cast<llvm::PointerType>(addrTy->getElementType()));
+    EmitARCStoreStrongCall(addr, null, /*ignored*/ true);
+    return;
+  }
+
+  llvm::Value *value = Builder.CreateLoad(addr);
+  EmitARCRelease(value, precise);
+}
+
 /// Store into a strong object.  Always calls this:
 ///   call void \@objc_storeStrong(i8** %addr, i8* %value)
 llvm::Value *CodeGenFunction::EmitARCStoreStrongCall(llvm::Value *addr,
@@ -2222,15 +2244,13 @@ void CodeGenFunction::EmitObjCMRRAutoreleasePoolPop(llvm::Value *Arg) {
 void CodeGenFunction::destroyARCStrongPrecise(CodeGenFunction &CGF,
                                               llvm::Value *addr,
                                               QualType type) {
-  llvm::Value *ptr = CGF.Builder.CreateLoad(addr, "strongdestroy");
-  CGF.EmitARCRelease(ptr, /*precise*/ true);
+  CGF.EmitARCDestroyStrong(addr, /*precise*/ true);
 }
 
 void CodeGenFunction::destroyARCStrongImprecise(CodeGenFunction &CGF,
                                                 llvm::Value *addr,
                                                 QualType type) {
-  llvm::Value *ptr = CGF.Builder.CreateLoad(addr, "strongdestroy");
-  CGF.EmitARCRelease(ptr, /*precise*/ false);  
+  CGF.EmitARCDestroyStrong(addr, /*precise*/ false);
 }
 
 void CodeGenFunction::destroyARCWeak(CodeGenFunction &CGF,
diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 65fa87f582..34fc8b9b35 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -2353,6 +2353,7 @@ public:
   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);
   llvm::Value *EmitARCAutorelease(llvm::Value *value);
   llvm::Value *EmitARCAutoreleaseReturnValue(llvm::Value *value);