From: John McCall <rjmccall@apple.com>
Date: Mon, 8 Apr 2013 23:27:49 +0000 (+0000)
Subject: Don't copy just to capture a strong block pointer under ARC.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4b9bcd667776932e9b4c84144a9e7e8d581ffa63;p=clang

Don't copy just to capture a strong block pointer under ARC.
It turns out that the optimizer can't eliminate this without extra
information, for which there's a separate bug.

rdar://13588325

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

diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 317d3c217d..ab9793132c 100644
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -753,6 +753,7 @@ llvm::Value *CodeGenFunction::EmitBlockLiteral(const CGBlockInfo &blockInfo) {
     if (capture.isConstant()) continue;
 
     QualType type = variable->getType();
+    CharUnits align = getContext().getDeclAlign(variable);
 
     // This will be a [[type]]*, except that a byref entry will just be
     // an i8**.
@@ -796,21 +797,21 @@ llvm::Value *CodeGenFunction::EmitBlockLiteral(const CGBlockInfo &blockInfo) {
     if (ci->isByRef()) {
       // Get a void* that points to the byref struct.
       if (ci->isNested())
-        src = Builder.CreateLoad(src, "byref.capture");
+        src = Builder.CreateAlignedLoad(src, align.getQuantity(),
+                                        "byref.capture");
       else
         src = Builder.CreateBitCast(src, VoidPtrTy);
 
       // Write that void* into the capture field.
-      Builder.CreateStore(src, blockField);
+      Builder.CreateAlignedStore(src, blockField, align.getQuantity());
 
     // If we have a copy constructor, evaluate that into the block field.
     } else if (const Expr *copyExpr = ci->getCopyExpr()) {
       if (blockDecl->isConversionFromLambda()) {
         // If we have a lambda conversion, emit the expression
         // directly into the block instead.
-        CharUnits Align = getContext().getTypeAlignInChars(type);
         AggValueSlot Slot =
-            AggValueSlot::forAddr(blockField, Align, Qualifiers(),
+            AggValueSlot::forAddr(blockField, align, Qualifiers(),
                                   AggValueSlot::IsDestructed,
                                   AggValueSlot::DoesNotNeedGCBarriers,
                                   AggValueSlot::IsNotAliased);
@@ -821,7 +822,27 @@ llvm::Value *CodeGenFunction::EmitBlockLiteral(const CGBlockInfo &blockInfo) {
 
     // If it's a reference variable, copy the reference into the block field.
     } else if (type->isReferenceType()) {
-      Builder.CreateStore(Builder.CreateLoad(src, "ref.val"), blockField);
+      llvm::Value *ref =
+        Builder.CreateAlignedLoad(src, align.getQuantity(), "ref.val");
+      Builder.CreateAlignedStore(ref, blockField, align.getQuantity());
+
+    // If this is an ARC __strong block-pointer variable, don't do a
+    // block copy.
+    //
+    // TODO: this can be generalized into the normal initialization logic:
+    // we should never need to do a block-copy when initializing a local
+    // variable, because the local variable's lifetime should be strictly
+    // contained within the stack block's.
+    } else if (type.getObjCLifetime() == Qualifiers::OCL_Strong &&
+               type->isBlockPointerType()) {
+      // Load the block and do a simple retain.
+      LValue srcLV = MakeAddrLValue(src, type, align);
+      llvm::Value *value = EmitLoadOfScalar(srcLV);
+      value = EmitARCRetainNonBlock(value);
+
+      // Do a primitive store to the block field.
+      LValue destLV = MakeAddrLValue(blockField, type, align);
+      EmitStoreOfScalar(value, destLV, /*init*/ true);
 
     // Otherwise, fake up a POD copy into the block field.
     } else {
@@ -839,8 +860,7 @@ llvm::Value *CodeGenFunction::EmitBlockLiteral(const CGBlockInfo &blockInfo) {
       ImplicitCastExpr l2r(ImplicitCastExpr::OnStack, type, CK_LValueToRValue,
                            &declRef, VK_RValue);
       EmitExprAsInit(&l2r, &blockFieldPseudoVar,
-                     MakeAddrLValue(blockField, type,
-                                    getContext().getDeclAlign(variable)),
+                     MakeAddrLValue(blockField, type, align),
                      /*captured by init*/ false);
     }
 
diff --git a/test/CodeGenObjC/arc-blocks.m b/test/CodeGenObjC/arc-blocks.m
index 3281b2aab8..c9ba2f6153 100644
--- a/test/CodeGenObjC/arc-blocks.m
+++ b/test/CodeGenObjC/arc-blocks.m
@@ -650,5 +650,44 @@ void test18(id x) {
 // CHECK-UNOPT-NEXT: ret void
 }
 
+// rdar://13588325
+void test19_sink(void (^)(int));
+void test19(void (^b)(void)) {
+// CHECK:    define void @test19(
+//   Prologue.
+// CHECK:      [[B:%.*]] = alloca void ()*,
+// CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
+// CHECK-NEXT: [[T0:%.*]] = bitcast void ()* {{%.*}} to i8*
+// CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retain(i8* [[T0]])
+// CHECK-NEXT: [[T2:%.*]] = bitcast i8* [[T1]] to void ()*
+// CHECK-NEXT: store void ()* [[T2]], void ()** [[B]]
+
+//   Block setup.  We skip most of this.  Note the bare retain.
+// CHECK-NEXT: [[SLOTREL:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
+// CHECK:      [[SLOT:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
+// CHECK-NEXT: [[T0:%.*]] = load void ()** [[B]],
+// CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
+// CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]])
+// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to void ()*
+// CHECK-NEXT: store void ()* [[T3]], void ()** [[SLOT]],
+//   Call.
+// CHECK-NEXT: [[T0:%.*]] = bitcast [[BLOCK_T]]* [[BLOCK]] to void (i32)*
+// CHECK-NEXT: call void @test19_sink(void (i32)* [[T0]])
+
+  test19_sink(^(int x) { b(); });
+
+//   Block teardown.
+// CHECK-NEXT: [[T0:%.*]] = load void ()** [[SLOTREL]]
+// CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
+// CHECK-NEXT: call void @objc_release(i8* [[T1]])
+
+//   Local cleanup.
+// CHECK-NEXT: [[T0:%.*]] = load void ()** [[B]]
+// CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
+// CHECK-NEXT: call void @objc_release(i8* [[T1]])
+
+// CHECK-NEXT: ret void
+}
+
 // CHECK: attributes [[NUW]] = { nounwind }
 // CHECK-UNOPT: attributes [[NUW]] = { nounwind }