r340408 - [CodeGen] Look at the type of a block capture field rather than the type
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 22 06:41:19 PDT 2018
Author: ahatanak
Date: Wed Aug 22 06:41:19 2018
New Revision: 340408
URL: http://llvm.org/viewvc/llvm-project?rev=340408&view=rev
Log:
[CodeGen] Look at the type of a block capture field rather than the type
of the captured variable when determining whether the capture needs
special handing when the block is copied or disposed.
This fixes bugs in the handling of variables captured by a block that is
nested inside a lambda that captures the variables by reference.
rdar://problem/43540889
Differential Revision: https://reviews.llvm.org/D51025
Added:
cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.mm
Removed:
cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.cpp
Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=340408&r1=340407&r2=340408&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Wed Aug 22 06:41:19 2018
@@ -557,6 +557,10 @@ static void computeBlockInfo(CodeGenModu
CharUnits align = CGM.getPointerAlign();
maxFieldAlign = std::max(maxFieldAlign, align);
+ // Since a __block variable cannot be captured by lambdas, its type and
+ // the capture field type should always match.
+ assert(getCaptureFieldType(*CGF, CI) == variable->getType() &&
+ "capture type differs from the variable type");
layout.push_back(BlockLayoutChunk(align, CGM.getPointerSize(),
Qualifiers::OCL_None, &CI,
CGM.VoidPtrTy, variable->getType()));
@@ -570,10 +574,11 @@ static void computeBlockInfo(CodeGenModu
continue;
}
+ QualType VT = getCaptureFieldType(*CGF, CI);
+
// If we have a lifetime qualifier, honor it for capture purposes.
// That includes *not* copying it if it's __unsafe_unretained.
- Qualifiers::ObjCLifetime lifetime =
- variable->getType().getObjCLifetime();
+ Qualifiers::ObjCLifetime lifetime = VT.getObjCLifetime();
if (lifetime) {
switch (lifetime) {
case Qualifiers::OCL_None: llvm_unreachable("impossible");
@@ -587,10 +592,10 @@ static void computeBlockInfo(CodeGenModu
}
// Block pointers require copy/dispose. So do Objective-C pointers.
- } else if (variable->getType()->isObjCRetainableType()) {
+ } else if (VT->isObjCRetainableType()) {
// But honor the inert __unsafe_unretained qualifier, which doesn't
// actually make it into the type system.
- if (variable->getType()->isObjCInertUnsafeUnretainedType()) {
+ if (VT->isObjCInertUnsafeUnretainedType()) {
lifetime = Qualifiers::OCL_ExplicitNone;
} else {
info.NeedsCopyDispose = true;
@@ -602,21 +607,18 @@ static void computeBlockInfo(CodeGenModu
} else if (CI.hasCopyExpr()) {
info.NeedsCopyDispose = true;
info.HasCXXObject = true;
- if (!variable->getType()->getAsCXXRecordDecl()->isExternallyVisible())
+ if (!VT->getAsCXXRecordDecl()->isExternallyVisible())
info.CapturesNonExternalType = true;
// So do C structs that require non-trivial copy construction or
// destruction.
- } else if (variable->getType().isNonTrivialToPrimitiveCopy() ==
- QualType::PCK_Struct ||
- variable->getType().isDestructedType() ==
- QualType::DK_nontrivial_c_struct) {
+ } else if (VT.isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct ||
+ VT.isDestructedType() == QualType::DK_nontrivial_c_struct) {
info.NeedsCopyDispose = true;
// And so do types with destructors.
} else if (CGM.getLangOpts().CPlusPlus) {
- if (const CXXRecordDecl *record =
- variable->getType()->getAsCXXRecordDecl()) {
+ if (const CXXRecordDecl *record = VT->getAsCXXRecordDecl()) {
if (!record->hasTrivialDestructor()) {
info.HasCXXObject = true;
info.NeedsCopyDispose = true;
@@ -626,7 +628,6 @@ static void computeBlockInfo(CodeGenModu
}
}
- QualType VT = getCaptureFieldType(*CGF, CI);
CharUnits size = C.getTypeSizeInChars(VT);
CharUnits align = C.getDeclAlign(variable);
@@ -1717,10 +1718,9 @@ static void findBlockCapturedManagedEnti
if (Capture.isConstant())
continue;
- auto CopyInfo =
- computeCopyInfoForBlockCapture(CI, Variable->getType(), LangOpts);
- auto DisposeInfo =
- computeDestroyInfoForBlockCapture(CI, Variable->getType(), LangOpts);
+ QualType VT = Capture.fieldType();
+ auto CopyInfo = computeCopyInfoForBlockCapture(CI, VT, LangOpts);
+ auto DisposeInfo = computeDestroyInfoForBlockCapture(CI, VT, LangOpts);
if (CopyInfo.first != BlockCaptureEntityKind::None ||
DisposeInfo.first != BlockCaptureEntityKind::None)
ManagedCaptures.emplace_back(CopyInfo.first, DisposeInfo.first,
Removed: cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.cpp?rev=340407&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.cpp (original)
+++ cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.cpp (removed)
@@ -1,23 +0,0 @@
-// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -o - %s | FileCheck %s
-
-// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
-// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
-// CHECK: %[[V1:.*]] = load i32*, i32** %[[V0]], align 8
-// CHECK: store i32* %[[V1]], i32** %[[BLOCK_CAPTURED0]], align 8
-// CHECK: %[[BLOCK_CAPTURED1:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK]], i32 0, i32 6
-// CHECK: %[[V2:.*]] = getelementptr inbounds %[[LAMBDA_CLASS]], %[[LAMBDA_CLASS]]* %[[THIS]], i32 0, i32 1
-// CHECK: %[[V3:.*]] = load i32*, i32** %[[V2]], align 8
-// CHECK: store i32* %[[V3]], i32** %[[BLOCK_CAPTURED1]], align 8
-
-void foo1(int &, int &);
-
-void block_in_lambda(int &s1, int &s2) {
- auto lambda = [&s1, &s2]() {
- auto block = ^{
- foo1(s1, s2);
- };
- block();
- };
-
- lambda();
-}
Added: cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.mm?rev=340408&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.mm (added)
+++ cfe/trunk/test/CodeGenObjCXX/block-nested-in-lambda.mm Wed Aug 22 06:41:19 2018
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+
+// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
+// CHECK: %[[V1:.*]] = load i32*, i32** %[[V0]], align 8
+// CHECK: store i32* %[[V1]], i32** %[[BLOCK_CAPTURED0]], align 8
+// CHECK: %[[BLOCK_CAPTURED1:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK]], i32 0, i32 6
+// CHECK: %[[V2:.*]] = getelementptr inbounds %[[LAMBDA_CLASS]], %[[LAMBDA_CLASS]]* %[[THIS]], i32 0, i32 1
+// CHECK: %[[V3:.*]] = load i32*, i32** %[[V2]], align 8
+// CHECK: store i32* %[[V3]], i32** %[[BLOCK_CAPTURED1]], align 8
+
+void foo1(int &, int &);
+
+void block_in_lambda(int &s1, int &s2) {
+ auto lambda = [&s1, &s2]() {
+ auto block = ^{
+ foo1(s1, s2);
+ };
+ block();
+ };
+
+ lambda();
+}
+
+namespace CaptureByReference {
+
+id getObj();
+void use(id);
+
+// Block copy/dispose helpers aren't needed because 'a' is captured by
+// reference.
+
+// CHECK-LABEL: define void @_ZN18CaptureByReference5test0Ev(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_1clEv"(
+// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>* %{{.*}}, i32 0, i32 4
+// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i64 }* @"__block_descriptor_40_e5_v8@?0ls32l8" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+
+void test0() {
+ id a = getObj();
+ [&]{ ^{ a = 0; }(); }();
+}
+
+// Block copy/dispose helpers shouldn't have to retain/release 'a' because it
+// is captured by reference.
+
+// CHECK-LABEL: define void @_ZN18CaptureByReference5test1Ev(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_2clEv"(
+// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 4
+// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @"__block_descriptor_56_8_32s40s_e5_v8@?0l" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_8_32s40s(
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 5
+// CHECK: %[[V5:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 5
+// CHECK: %[[BLOCKCOPY_SRC:.*]] = load i8*, i8** %[[V4]], align 8
+// CHECK: store i8* null, i8** %[[V5]], align 8
+// CHECK: call void @objc_storeStrong(i8** %[[V5]], i8* %[[BLOCKCOPY_SRC]])
+// CHECK: %[[V6:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 6
+// CHECK: %[[V7:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 6
+// CHECK: %[[BLOCKCOPY_SRC2:.*]] = load i8*, i8** %[[V6]], align 8
+// CHECK: store i8* null, i8** %[[V7]], align 8
+// CHECK: call void @objc_storeStrong(i8** %[[V7]], i8* %[[BLOCKCOPY_SRC2]])
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: ret void
+
+// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block_8_32s40s(
+// CHECK: %[[V2:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 5
+// CHECK: %[[V3:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 6
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: call void @objc_storeStrong(i8** %[[V3]], i8* null)
+// CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null)
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: ret void
+
+void test1() {
+ id a = getObj(), b = getObj(), c = getObj();
+ [&a, b, c]{ ^{ a = 0; use(b); use(c); }(); }();
+}
+
+}
More information about the cfe-commits
mailing list