[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

In https://reviews.llvm.org/D41050#954668, @rjmccall wrote:

> In https://reviews.llvm.org/D41050#953917, @danzimm wrote:
>
> > Change tests to use non-O2 generated IR. It looks like the combined 
> > objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls 
> > annihilate each other and we just get a call/ret.
>
>
> Is that really happening at -O0?  Maybe you need to add -disable-llvm-optzns 
> to the test line if so.


Unfortunately it looks like adding `-disable-llvm-optzns` and/or 
`-disable-llvm-passes` (I also manually added `-O0` since I don't know what 
clang defaults to) doesn't generate the explicit call to 
`objc_retainAutoreleasedReturnValue`. Should we add a flag to disable that 
merging?


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

I just dug into how the ARC optimization pass is invoked... it really shouldn't 
be invoked if `-disable-llvm-passes` is passed (or `-disable-llvm-optzns` which 
appears to just alias the first). Can you verify that my command is sane:

  /Users/danzimm/oss/build/bin/clang -cc1 -internal-isystem 
/Users/danzimm/oss/build/lib/clang/6.0.0/include -nostdsysteminc -triple 
x86_64-apple-macosx10.12.0 -disable-llvm-passes -emit-llvm -fblocks -fobjc-arc 
-std=c++11 -O0 -o - 
/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 126892.
danzimm added a comment.

Pass -disable-llvm-passes with -O3 so that we can test that the IR we generate 
actually is generated


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm 
-disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 
-o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* 
@"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]]) [[A0:#[0-9]+]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]]) [[A0]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]]) [[A0:#[0-9]+]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]]) [[A0]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 126893.
danzimm added a comment.

Remove unnecessary checks from tests (sorry for unbatched updates)


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm 
-disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 
-o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-14 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@dexonsmith Here are my results after passing those extra flags with `-O3` 
(/Users/danzimm/oss/build/bin/clang -cc1 -internal-isystem 
/Users/danzimm/oss/build/lib/clang/6.0.0/include -nostdsysteminc -triple 
x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O3 -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -mllvm -debug-pass=Executions -o 
~/foo.ll 
/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm):

  Pass Arguments:  -tti
  Target Transform Information
FunctionPass Manager
  Pass Arguments:  -tti
  Target Transform Information
ModulePass Manager
  Print Module IR
  [2017-12-14 09:47:12.252472000] 0x7f87b90016f0   Executing Pass 'Print Module 
IR' on Module 
'/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm'...
  [2017-12-14 09:47:12.254702000] 0x7f87b90016f0Freeing Pass 'Print Module 
IR' on Module 
'/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm'...
  Pass Arguments:  -tti
  Target Transform Information
ModulePass Manager

and with `-O0` (/Users/danzimm/oss/build/bin/clang -cc1 -internal-isystem 
/Users/danzimm/oss/build/lib/clang/6.0.0/include -nostdsysteminc -triple 
x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O0 -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -mllvm -debug-pass=Executions -o 
~/foo.ll 
/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm):

  Pass Arguments:  -tti
  Target Transform Information
FunctionPass Manager
  Pass Arguments:  -tti
  Target Transform Information
ModulePass Manager
  Print Module IR
  [2017-12-14 09:47:20.884147000] 0x7ff71ed097d0   Executing Pass 'Print Module 
IR' on Module 
'/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm'...
  [2017-12-14 09:47:20.886182000] 0x7ff71ed097d0Freeing Pass 'Print Module 
IR' on Module 
'/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm'...
  Pass Arguments:  -tti
  Target Transform Information
ModulePass Manager


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-14 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

Also, I don't have commit access. Could someone else commit this for me?


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In https://bugs.llvm.org/show_bug.cgi?id=41206 we observe bad codegen when 
embedding a non-trivial C struct within a C struct. This is due to the fact 
that name mangling for non-trivial structs marks the two structs as identical. 
This diff contains a potential fix for this issue. I've included a test that 
exercises this for two scenarios. Please comment if there're any other 
scenarios I should consider.

Test Plan: I've included a unit test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59873

Files:
  clang/lib/CodeGen/CGNonTrivialStruct.cpp


Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -139,8 +139,8 @@
 //  ::=  ["_" ]
 //  ::= +
 //  ::=  | 
-//  ::=  |  
|
-//   
+//  ::= "_S"  |
+//| 
 //  ::= "_AB"  "s"  "n"
 //  "_AE"
 //  ::= 
@@ -175,6 +175,7 @@
   void visitStruct(QualType QT, const FieldDecl *FD,
CharUnits CurStructOffset) {
 CharUnits FieldOffset = CurStructOffset + asDerived().getFieldOffset(FD);
+appendStr("_S");
 asDerived().visitStructFields(QT, FieldOffset);
   }
 


Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -139,8 +139,8 @@
 //  ::=  ["_" ]
 //  ::= +
 //  ::=  | 
-//  ::=  |  |
-//   
+//  ::= "_S"  |
+//| 
 //  ::= "_AB"  "s"  "n"
 //  "_AE"
 //  ::= 
@@ -175,6 +175,7 @@
   void visitStruct(QualType QT, const FieldDecl *FD,
CharUnits CurStructOffset) {
 CharUnits FieldOffset = CurStructOffset + asDerived().getFieldOffset(FD);
+appendStr("_S");
 asDerived().visitStructFields(QT, FieldOffset);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@lebedev.ri right, sorry about that- I prematurely diff'd (got a few terminals 
crossed and thought I was finished with the test already). I will be amending 
with a test and a few other test fixes shortly. Sorry about the 
miscommunication :/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59873/new/

https://reviews.llvm.org/D59873



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 192436.
danzimm added a comment.

I forgot to add the test originally, this update contains a test and updates to 
old tests to make them pass


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59873/new/

https://reviews.llvm.org/D59873

Files:
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
  clang/test/CodeGenObjC/strong-in-c-struct.m

Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,12 +89,12 @@
 // CHECK: define void @test_constructor_destructor_StrongOuter()
 // CHECK: %[[T:.*]] = alloca %[[STRUCT_STRONGOUTER:.*]], align 8
 // CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V0]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V0]])
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__default_constructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -117,7 +117,7 @@
 // CHECK: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V4]], i8 0, i64 8, i1 false)
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__destructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -149,12 +149,12 @@
 // CHECK: %[[V0:.*]] = load %[[STRUCT_STRONGOUTER]]*, %[[STRUCT_STRONGOUTER]]** %[[S_ADDR]], align 8
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
 // CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[V0]] to i8**
-// CHECK: call void @__copy_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: call void @__copy_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[V1]], i8** %[[V2]])
 // CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V3]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V3]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
@@ -208,7 +208,7 @@
   StrongOuter t = *s;
 }
 
-/// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+/// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
@@ -231,15 +231,15 @@
 // CHECK: define void @test_move_constructor_StrongOuter()
 // CHECK: %[[T1:.*]] = getelementptr inbounds %[[STRUCT_BLOCK_BYREF_T:.*]], %[[STRUCT_BLOCK_BYREF_T]]* %{{.*}}, i32 0, i32 7
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T1]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: %[[T2:.*]] = getelementptr inbounds %[[STRUCT_BLOCK_BYREF_T]], %[[STRUCT_BLOCK_BYREF_T]]* %{{.*}}, i32 0, i32 7
 // CHECK: %[[V9:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T2]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V9]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V9]])
 
 // CHECK: define internal void @__Block_byref_object_copy_(i8*, i8*)
-// CHECK: call void @__move_constructor_8_8_t0w16_s16_s24_t32w8(
+// CHECK: call void @__move_constructor_8_8_S_t0w16_s16_s24_t32w8(
 
-// CHECK: define linkonce_odr hidden void @__move_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// CHECK: define linkonce_odr hidden void @__move_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_AD

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@smeenai please feel free add any reviewers that I might've missed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59873/new/

https://reviews.llvm.org/D59873



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-28 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 192652.
danzimm added a comment.

Add a third level to ensure nontrivial structs within structs within structs 
works (this suggests that N embeddings works too). Also change the invocation 
of the test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59873/new/

https://reviews.llvm.org/D59873

Files:
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
  clang/test/CodeGenObjC/strong-in-c-struct.m

Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,12 +89,12 @@
 // CHECK: define void @test_constructor_destructor_StrongOuter()
 // CHECK: %[[T:.*]] = alloca %[[STRUCT_STRONGOUTER:.*]], align 8
 // CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V0]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V0]])
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__default_constructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -117,7 +117,7 @@
 // CHECK: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V4]], i8 0, i64 8, i1 false)
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__destructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -149,12 +149,12 @@
 // CHECK: %[[V0:.*]] = load %[[STRUCT_STRONGOUTER]]*, %[[STRUCT_STRONGOUTER]]** %[[S_ADDR]], align 8
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
 // CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[V0]] to i8**
-// CHECK: call void @__copy_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: call void @__copy_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[V1]], i8** %[[V2]])
 // CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V3]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V3]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
@@ -208,7 +208,7 @@
   StrongOuter t = *s;
 }
 
-/// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+/// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
@@ -231,15 +231,15 @@
 // CHECK: define void @test_move_constructor_StrongOuter()
 // CHECK: %[[T1:.*]] = getelementptr inbounds %[[STRUCT_BLOCK_BYREF_T:.*]], %[[STRUCT_BLOCK_BYREF_T]]* %{{.*}}, i32 0, i32 7
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T1]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: %[[T2:.*]] = getelementptr inbounds %[[STRUCT_BLOCK_BYREF_T]], %[[STRUCT_BLOCK_BYREF_T]]* %{{.*}}, i32 0, i32 7
 // CHECK: %[[V9:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T2]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V9]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V9]])
 
 // CHECK: define internal void @__Block_byref_object_copy_(i8*, i8*)
-// CHECK: call void @__move_constructor_8_8_t0w16_s16_s24_t32w8(
+// CHECK: call void @__move_constructor_8_8_S_t0w16_s16_s24_t32w8(
 
-// CHECK: define linkonce_odr hidden void @__move_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// CHECK: define linkonce_odr hidden void @__move_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-28 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@smeenai good idea on the third level!

Yep, I'll need somebody to commit this for me, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59873/new/

https://reviews.llvm.org/D59873



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52664: Update CMakeLists.txt snippet so that example compiles

2018-09-28 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm created this revision.
danzimm added a reviewer: modocache.
Herald added a subscriber: cfe-commits.

Previous to this the example didn't work out of the box, it seems some cmake 
config changed between when this was written and now.


Repository:
  rC Clang

https://reviews.llvm.org/D52664

Files:
  docs/LibASTMatchersTutorial.rst


Index: docs/LibASTMatchersTutorial.rst
===
--- docs/LibASTMatchersTutorial.rst
+++ docs/LibASTMatchersTutorial.rst
@@ -113,6 +113,7 @@
 LoopConvert.cpp
 )
   target_link_libraries(loop-convert
+PRIVATE
 clangTooling
 clangBasic
 clangASTMatchers


Index: docs/LibASTMatchersTutorial.rst
===
--- docs/LibASTMatchersTutorial.rst
+++ docs/LibASTMatchersTutorial.rst
@@ -113,6 +113,7 @@
 LoopConvert.cpp
 )
   target_link_libraries(loop-convert
+PRIVATE
 clangTooling
 clangBasic
 clangASTMatchers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52664: Update CMakeLists.txt snippet so that example compiles

2018-10-01 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@steveire can you land this for me? I don't have commit access


Repository:
  rC Clang

https://reviews.llvm.org/D52664



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

2017-12-09 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm created this revision.
Herald added subscribers: cfe-commits, kosarev.

Clang has a pretty cool feature right now that will allow you to use a lambda 
as a block. Unfortunately there's a bug in this conversion if the return value 
of the block is an ObjC object and arc is enabled -- the return value gets 
over-released. An example of this causing a crash that shouldn't occur can be 
seen below:

  typedef id (^Go)();
  
  int main() {
Go g = []() -> id {
  return [NSObject new];
};
id sss = nil;
@autoreleasepool {
  id ss = g();
  sss = ss;
}
NSLog(@"%@", sss);
return 0;
  }

It looks like when a lambda is auto-converted to a block it generates a block 
that just wraps the lambda and forwards all args to it and the return value of 
the lambda back to be the return value of the block. Currently if the block 
returns an ObjC object both the lambda that's generated and the block that 
wraps it autorelease the return value, which effectively over-releases the 
return value. There are a few options that I considered and one that stuck out 
to me:

1. Add a retain call on the return value after invoking the enclosed lambda -- 
this way we maintain proper retain count
2. Stop autoreleasing the return value of the lambda
3. Stop autoreleasing the return value of the block

I opted against the first case since it would mean unnecessary extra reference 
count churn. I didn't see an easy way to implement the second option as the 
enclosing lambda is generated as a global function definition, without the 
metadata that a block encloses it (and thus there's no easy way that I could 
see to differentiate the lambda-converted-to-block function definition from 
other global function definitions). Thus I came to the third option which is 
easily implementable since the codegen for blocks must recognize whether or not 
it encloses a lambda (as it changes what it generates if it does).

The test I wrote relies on the optimizer inlining the lambda invocation -- I'm 
not sure if that's an ok assumption to make. The IR generated previous to this 
patch is kind of fun:

  define internal i8* @___Z5test0P11objc_object_block_invoke(i8* nocapture 
readonly %.block_descriptor) #0 {
  entry:
%block.capture.addr = getelementptr inbounds i8, i8* %.block_descriptor, 
i64 32
%0 = bitcast i8* %block.capture.addr to i8**
%1 = load i8*, i8** %0, align 8, !tbaa !7
%2 = tail call i8* @objc_retain(i8* %1) #2
%3 = tail call i8* @objc_autoreleaseReturnValue(i8* %1) #2
%4 = tail call i8* @objc_autoreleaseReturnValue(i8* %1) #2
ret i8* %1
  }


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGBlocks.cpp
  test/CodeGenObjCXX/arc-block-lambda-conversion.mm


Index: test/CodeGenObjCXX/arc-block-lambda-conversion.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-block-lambda-conversion.mm
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -O2 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -1450,9 +1450,12 @@
   llvm::BasicBlock::iterator entry_ptr = Builder.GetInsertPoint();
   --entry_ptr;
 
-  if (IsLambdaConversionToBlock)
+  if (IsLambdaConversionToBlock) {
+// The lambda that's generated will emit a call to
+// objc_autoreleaseReturnValue for us if necessary
+AutoreleaseResult = false;
 EmitLambdaBlockInvokeBody();
-  else {
+  } else {
 PGO.assignRegionCounters(GlobalDecl(blockDecl), fn);
 incrementProfileCounter(blockDecl->getBody());
 EmitStmt(blockDecl->getBody());


Index: test/CodeGenObjCXX/arc-block-lambda-conversion.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-block-lambda-conversion.mm
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -O2 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* [[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGBlocks.cpp
=

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

2017-12-12 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@rjmccall ah right, good point -- I think it's ok to elide that 
objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue pair in this 
case though, since all of this is generated within clang itself (and thus we 
can make the guarentee that the object will properly live until the return of 
the enclosing block) -- it reduces retain count churn this way too


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

2017-12-12 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

Or do we need to abide by those semantics strictly here? Could you expand on 
why that is, if that's the case?


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

2017-12-12 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 126628.
danzimm added a comment.

Call objc_retainAutoreleasedReturnValue after invoking a wrapped lambda


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -O2 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -1450,9 +1450,9 @@
   llvm::BasicBlock::iterator entry_ptr = Builder.GetInsertPoint();
   --entry_ptr;
 
-  if (IsLambdaConversionToBlock)
+  if (IsLambdaConversionToBlock) {
 EmitLambdaBlockInvokeBody();
-  else {
+  } else {
 PGO.assignRegionCounters(GlobalDecl(blockDecl), fn);
 incrementProfileCounter(blockDecl->getBody());
 EmitStmt(blockDecl->getBody());


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -O2 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* [[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* [[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -1450,9 +1450,9 @@
   llvm::BasicBlock::iterator entry_ptr = Builder.GetInsertPoint();
   --entry_ptr;
 
-  if (IsLambdaConversionToBlock)
+  if (IsLambdaConversionToBlock) {
 EmitLambdaBlockInvokeBody();
-  else {
+  } else {
 PGO.assignRegionCounters(GlobalDecl(blockDecl), fn);
 incrementProfileCounter(blockDecl->getBody());
 EmitStmt(blockDecl->getBody());
__

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

2017-12-12 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

@rjmccall aha, right - thanks for explaining that to me (sorry for the bad 
logic I had earlier).

It turns out this was also broken for a lambda that was auto-converted to a 
function pointer who returned an ObjC object. I changed the test case to 
reflect the more general situation, but I'm still unsure about the quality of 
the tests. Is it ok to rely on optimization passes to inline certain calls?


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

2017-12-12 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 126633.
danzimm added a comment.

Remove unnecessary change of braces


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -O2 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -O2 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* [[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: {{%.*}} = tail call i8* @objc_retain(i8* [[T0:%.*]])
+  // CHECK-NEXT: {{%.*}} = tail call i8* @objc_autoreleaseReturnValue(i8* [[T0]])
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm updated this revision to Diff 126763.
danzimm added a comment.

Change tests to use non-O2 generated IR. It looks like the combined 
objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls annihilate 
each other and we just get a call/ret.


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* 
@"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits 
danzimm added a comment.

Ah, just found test/Transforms/ObjCARC/rv.ll test3:

  ; Delete a redundant retainRV,autoreleaseRV when forwaring a call result
  ; directly to a return value.
  
  ; CHECK-LABEL: define i8* @test3(
  ; CHECK: call i8* @returner()
  ; CHECK-NEXT: ret i8* %call
  define i8* @test3() {
  entry:
%call = call i8* @returner()
%0 = call i8* @objc_retainAutoreleasedReturnValue(i8* %call) nounwind
%1 = call i8* @objc_autoreleaseReturnValue(i8* %0) nounwind
ret i8* %1
  }


Repository:
  rC Clang

https://reviews.llvm.org/D41050



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits