[CodeGen][ObjC] Don't retain/release capture objects at block creation that are const-qualified

[ahatanak]

Summary

When a block captures an ObjC object pointer, clang emits a retain/release pair to prevent prematurely destroying the object the pointer points to before the block is called or copied.

When the captured object pointer is const-qualified, we can avoid emitting the retain/release pair since the pointer variable cannot be modified in the scope in which the block literal is introduced.

void test(const id x) {
  callee(^{ (void)x; });
}

This patch implements that optimization.

rdar://problem/28894510

[ahatanak]

There is a comment in test/CodeGenObjC/arc-foreach.m that states the variable introduced by fast enumeration should be retained, but I don't think that is needed?

[rjmccall]

I agree, this seems reasonable. LGTM.

lib/CodeGen/CGBlocks.cpp

for (const auto &CI : block->captures()) {
     QualType VT = getCaptureFieldType(CGF, CI);
     QualType::DestructionKind dtorKind = VT.isDestructedType();
     if (dtorKind == QualType::DK_none) continue;
 
     CodeGenFunction::Destroyer *destroyer;
 
     // Block captures count as local values and have imprecise semantics.
     // They also can't be arrays, so need to worry about that.
-    if (dtorKind == QualType::DK_objc_strong_lifetime) {
+    //
+    // For const-qualified captures, emit clang.arc.use to ensure the captured
+    // object doesn't get released while we are still depending on its validity
+    // within the block.
+    if (VT.isConstQualified() && VT.getObjCLifetime() == Qualifiers::OCL_Strong)
+      destroyer = CodeGenFunction::emitARCIntrinsicUse;
+    else if (dtorKind == QualType::DK_objc_strong_lifetime) {
       destroyer = CodeGenFunction::destroyARCStrongImprecise;
     } else {
       destroyer = CGF.getDestroyer(dtorKind);
     }
 
     // GEP down to the address.
     Address addr = CGF.Builder.CreateStructGEP(blockInfo.LocalAddress,
                                                capture.getIndex(),
// If we have a copy constructor, evaluate that into the block field.
       } else {
         EmitSynthesizedCXXCopyCtor(blockField, src, copyExpr);
       }
 
     // If it's a reference variable, copy the reference into the block field.
     } else if (type->isReferenceType()) {
       Builder.CreateStore(src.getPointer(), blockField);
 
+    // If type is const-qualified, copy the value into the block field.
+    } else if (type.isConstQualified() &&
+               type.getObjCLifetime() == Qualifiers::OCL_Strong) {
+      llvm::Value *value = Builder.CreateLoad(src, "captured");
+      Builder.CreateStore(value, blockField);
+
     // 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 &&

lib/CodeGen/CGObjC.cpp

 }
 
 void CodeGenFunction::destroyARCWeak(CodeGenFunction &CGF,
                                      Address addr,
                                      QualType type) {
   CGF.EmitARCDestroyWeak(addr);
 }
 
+void CodeGenFunction::emitARCIntrinsicUse(CodeGenFunction &CGF, Address addr,
+                                          QualType type) {
+  llvm::Value *value = CGF.Builder.CreateLoad(addr);
+  CGF.EmitARCIntrinsicUse(value);
+}
+
 namespace {
   struct CallObjCAutoreleasePoolObject final : EHScopeStack::Cleanup {
     llvm::Value *Token;
 
     CallObjCAutoreleasePoolObject(llvm::Value *token) : Token(token) {}
 
     void Emit(CodeGenFunction &CGF, Flags flags) override {
       CGF.EmitObjCAutoreleasePoolPop(Token);

lib/CodeGen/CodeGenFunction.h

public:
   llvm::Value *EmitARCRetainAutoreleaseScalarExpr(const Expr *expr);
   llvm::Value *EmitARCUnsafeUnretainedScalarExpr(const Expr *expr);
 
   void EmitARCIntrinsicUse(ArrayRef<llvm::Value*> values);
 
   static Destroyer destroyARCStrongImprecise;
   static Destroyer destroyARCStrongPrecise;
   static Destroyer destroyARCWeak;
+  static Destroyer emitARCIntrinsicUse;
 
   void EmitObjCAutoreleasePoolPop(llvm::Value *Ptr); 
   llvm::Value *EmitObjCAutoreleasePoolPush();
   llvm::Value *EmitObjCMRRAutoreleasePoolPush();
   void EmitObjCAutoreleasePoolCleanup(llvm::Value *Ptr);
   void EmitObjCMRRAutoreleasePoolPop(llvm::Value *Ptr); 
 
   /// \brief Emits a reference binding to the passed in expression.

test/CodeGenObjC/arc-blocks.m

 // CHECK:      [[SELF:%.*]] = alloca [[TEST8:%.*]]*,
 // CHECK-NEXT: alloca i8*
 // CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
 // CHECK: store
 // CHECK-NEXT: store
 // CHECK:      [[D0:%.*]] = getelementptr inbounds [[BLOCK_T]], [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
 // CHECK:      [[T0:%.*]] = getelementptr inbounds [[BLOCK_T]], [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
 // CHECK-NEXT: [[T1:%.*]] = load [[TEST8]]*, [[TEST8]]** [[SELF]],
-// CHECK-NEXT: [[T2:%.*]] = bitcast [[TEST8]]* [[T1]] to i8*
-// CHECK-NEXT: [[T3:%.*]] = call i8* @objc_retain(i8* [[T2]])
-// CHECK-NEXT: [[T4:%.*]] = bitcast i8* [[T3]] to [[TEST8]]*
-// CHECK-NEXT: store [[TEST8]]* [[T4]], [[TEST8]]** [[T0]]
+// CHECK-NEXT: store %0* [[T1]], %0** [[T0]]
 // CHECK-NEXT: bitcast [[BLOCK_T]]* [[BLOCK]] to
 // CHECK: call void @test8_helper(
-// CHECK-NEXT: [[T1:%.*]] = load [[TEST8]]*, [[TEST8]]** [[D0]]
-// CHECK-NEXT: [[T2:%.*]] = bitcast [[TEST8]]* [[T1]] to i8*
-// CHECK-NEXT: call void @objc_release(i8* [[T2]])
+// CHECK-NEXT: [[T2:%.*]] = load [[TEST8]]*, [[TEST8]]** [[D0]]
+// CHECK-NEXT: call void (...) @clang.arc.use([[TEST8]]* [[T2]])
 // CHECK: ret void
 
   extern void test8_helper(void (^)(void));
   test8_helper(^{ (void) self; });
 }
 @end
 
 id test9(void) {

 //   Local cleanup.
 // CHECK-NEXT: [[T0:%.*]] = load void ()*, void ()** [[B]]
 // CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
 // CHECK-NEXT: call void @objc_release(i8* [[T1]])
 
 // CHECK-NEXT: ret void
 }
 
+// CHECK-LABEL: define void @test20(
+// CHECK: [[XADDR:%.*]] = alloca i8*
+// CHECK-NEXT: [[BLOCK:%.*]] = alloca <[[BLOCKTY:.*]]>
+// CHECK-NEXT: [[RETAINEDX:%.*]] = call i8* @objc_retain(i8* %{{.*}})
+// CHECK-NEXT: store i8* [[RETAINEDX]], i8** [[XADDR]]
+// CHECK-NEXT: [[CAPTUREFIELD:%.*]] = getelementptr inbounds <[[BLOCKTY]]>, <[[BLOCKTY]]>* [[BLOCK]], i32 0, i32 5
+// CHECK: [[BLOCKCAPTURED:%.*]] = getelementptr inbounds <[[BLOCKTY]]>, <[[BLOCKTY]]>* [[BLOCK]], i32 0, i32 5
+// CHECK: [[CAPTURED:%.*]] = load i8*, i8** [[XADDR]]
+// CHECK: store i8* [[CAPTURED]], i8** [[BLOCKCAPTURED]]
+// CHECK: [[CAPTURE:%.*]] = load i8*, i8** [[CAPTUREFIELD]]
+// CHECK-NEXT: call void (...) @clang.arc.use(i8* [[CAPTURE]])
+// CHECK-NEXT: [[X:%.*]] = load i8*, i8** [[XADDR]]
+// CHECK-NEXT: call void @objc_release(i8* [[X]])
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @__copy_helper_block
+// CHECK: [[BLOCKSOURCE:%.*]] = bitcast i8* %{{.*}} to <[[BLOCKTY]]>*
+// CHECK: [[CAPTUREFIELD:%.*]] = getelementptr inbounds <[[BLOCKTY]]>, <[[BLOCKTY]]>* [[BLOCKSOURCE]], i32 0, i32 5
+// CHECK: [[BLOCKCOPYSRC:%.*]] = load i8*, i8** [[CAPTUREFIELD]]
+// CHECK: call i8* @objc_retain(i8* [[BLOCKCOPYSRC]])
+
+void test20_callee(void (^)());
+void test20(const id x) {
+  test20_callee(^{ (void)x; });
+}
+
 // CHECK: attributes [[NUW]] = { nounwind }
 // CHECK-UNOPT: attributes [[NUW]] = { nounwind }

test/CodeGenObjC/arc-foreach.m

 // CHECK-LP64-NEXT: [[T1:%.*]] = load i8**, i8*** [[T0]]
 // CHECK-LP64-NEXT: [[T2:%.*]] = getelementptr i8*, i8** [[T1]], i64
 // CHECK-LP64-NEXT: [[T3:%.*]] = load i8*, i8** [[T2]]
 // CHECK-LP64-NEXT: store i8* [[T3]], i8** [[X]]
 
 // CHECK-LP64:      [[D0:%.*]] = getelementptr inbounds [[BLOCK_T]], [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
 // CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[BLOCK_T]], [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
 // CHECK-LP64-NEXT: [[T1:%.*]] = load i8*, i8** [[X]]
-// CHECK-LP64-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]])
-// CHECK-LP64-NEXT: store i8* [[T2]], i8** [[T0]]
-// CHECK-LP64-NEXT: [[T1:%.*]] = bitcast [[BLOCK_T]]* [[BLOCK]] 
-// CHECK-LP64: call void @use_block(
-// CHECK-LP64-NEXT: call void @objc_storeStrong(i8** [[D0]], i8* null)
+// CHECK-LP64-NEXT: store i8* [[T1]], i8** [[T0]]
+// CHECK-LP64-NEXT: [[BLOCK1:%.*]] = bitcast [[BLOCK_T]]* [[BLOCK]]
+// CHECK-LP64-NEXT: call void @use_block(void ()* [[BLOCK1]])
+// CHECK-LP64-NEXT: [[CAPTURE:%.*]] = load i8*, i8** [[D0]]
+// CHECK-LP64: call void (...) @clang.arc.use(i8* [[CAPTURE]])
 
 // CHECK-LP64:      [[T0:%.*]] = load i8*, i8** @OBJC_SELECTOR_REFERENCES_
 // CHECK-LP64-NEXT: [[T1:%.*]] = bitcast [[ARRAY_T]]* [[SAVED_ARRAY]] to i8*
 // CHECK-LP64-NEXT: [[SIZE:%.*]] = call i64 bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to i64 (i8*, i8*, [[STATE_T]]*, [16 x i8*]*, i64)*)(i8* [[T1]], i8* [[T0]], [[STATE_T]]* [[STATE]], [16 x i8*]* [[BUFFER]], i64 16)
 
 // Release the array.
 // CHECK-LP64:      [[T0:%.*]] = bitcast [[ARRAY_T]]* [[SAVED_ARRAY]] to i8*
 // CHECK-LP64-NEXT: call void @objc_release(i8* [[T0]])

 
 // CHECK-LP64-LABEL: define internal void @"\01-[I1 foo2]"(
 // CHECK-LP64:         [[SELF_ADDR:%.*]] = alloca [[TY:%.*]]*,
 // CHECK-LP64:         [[BLOCK:%.*]] = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, [[TY]]* }>,
 // CHECK-LP64:         store [[TY]]* %self, [[TY]]** [[SELF_ADDR]]
 // CHECK-LP64:         [[T0:%.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, [[TY]]* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, [[TY]]* }>* [[BLOCK]], i32 0, i32 5
 // CHECK-LP64:         [[BC:%.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, [[TY]]* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, [[TY]]* }>* [[BLOCK]], i32 0, i32 5
 // CHECK-LP64:         [[T1:%.*]] = load [[TY]]*, [[TY]]** [[SELF_ADDR]]
-// CHECK-LP64:         [[T2:%.*]] = bitcast [[TY]]* [[T1]] to i8*
-// CHECK-LP64:         [[T3:%.*]] = call i8* @objc_retain(i8* [[T2]])
-// CHECK-LP64:         [[T4:%.*]] = bitcast i8* [[T3]] to [[TY]]*
-// CHECK-LP64:         store [[TY]]* [[T4]], [[TY]]** [[BC]]
+// CHECK-LP64:         store [[TY]]* [[T1]], [[TY]]** [[BC]]
 
-// CHECK-LP64:         [[T5:%.*]] = bitcast [[TY]]** [[T0]] to i8**
-// CHECK-LP64:         call void @objc_storeStrong(i8** [[T5]], i8* null)
+// CHECK-LP64:         [[T5:%.*]] = load [[TY]]*, [[TY]]** [[T0]]
+// CHECK-LP64:         call void (...) @clang.arc.use([[TY]]* [[T5]])
 // CHECK-LP64:         switch i32 {{%.*}}, label %[[UNREACHABLE:.*]] [
 // CHECK-LP64-NEXT:      i32 0, label %[[CLEANUP_CONT:.*]]
 // CHECK-LP64-NEXT:      i32 2, label %[[FORCOLL_END:.*]]
 // CHECK-LP64-NEXT:    ]
 
 // CHECK-LP64:       {{^|:}}[[CLEANUP_CONT]]
 // CHECK-LP64-NEXT:    br label %[[FORCOLL_END]]
 
 // CHECK-LP64:       {{^|:}}[[FORCOLL_END]]
 // CHECK-LP64-NEXT:    ret void
 
 // CHECK-LP64:       {{^|:}}[[UNREACHABLE]]
 // CHECK-LP64-NEXT:    unreachable
 
 // CHECK-LP64: attributes [[NUW]] = { nounwind }