[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
This revision was automatically updated to reflect the committed changes. Closed by commit rL345903: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53674?vs=172051&id=172258#toc Repository: rL LLVM https://reviews.llvm.org/D53674 Files: cfe/trunk/lib/CodeGen/CGObjC.cpp cfe/trunk/test/CodeGenObjCXX/arc-constexpr.mm Index: cfe/trunk/test/CodeGenObjCXX/arc-constexpr.mm === --- cfe/trunk/test/CodeGenObjCXX/arc-constexpr.mm +++ cfe/trunk/test/CodeGenObjCXX/arc-constexpr.mm @@ -1,18 +1,51 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag +@class NSString; -// CHECK: define void @_Z5test1v +// CHECK-LABEL: define void @_Z5test1v // CHECK: %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]* // CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] // CHECK: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* // CHECK: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]] // CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]] // CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null) - -@class NSString; - void test1() { constexpr NSString *S = @"abc"; } + +// CHECK-LABEL: define void @_Z5test2v +// CHECK: %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]] +// CHECK: %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]] +// CHECK: %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null) +// CHECK: %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null) +void test2() { + constexpr NSString *Const = @"abc"; + // In IR RefConst should be initialized with Const initializer instead of + // reading from variable. + NSString* RefConst = Const; +} + +// CHECK-LABEL: define void @_Z5test3v +// CHECK: %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK: store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null) +// CHECK: %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]]) +void test3() { + __weak constexpr NSString *WeakConst = @"abc"; + NSString* RefWeakConst = WeakConst; +} Index: cfe/trunk/lib/CodeGen/CGObjC.cpp === --- cfe/trunk/lib/CodeGen/CGObjC.cpp +++ cfe/trunk/lib/CodeGen/CGObjC.cpp @@ -2447,27 +2447,36 @@ EHStack.pushCleanup(NormalCleanup, Ptr); } -static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, - LValue lvalue, - QualType type) { - switch (type.getObjCLifetime()) { +static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime lifetime) { + switch (lifetime) { case Qualifiers::OCL_None: case Qualifiers::OCL_ExplicitNone: case Qualifiers::OCL_Strong: case Qualifiers::OCL_Autoreleasing: -return TryEmitResult(CGF.EmitLoadOfLValue(lvalue, - SourceLocation()).getScalarVal(), - false); +return true; case Qualifiers::OCL_Weak: -return TryEmitResult(CGF.EmitARCLoadWeakRetained(lvalue.getAddress()), - true); +return false; } llvm_unreachable("impossible lifetime!"); } static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, + LValue lvalue, +
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai added a comment. Thanks for the review, John. I'll update the comment with TODO and commit. https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai updated this revision to Diff 172051. vsapsai added a comment. Exclude commits tracked in a different review. https://reviews.llvm.org/D53674 Files: clang/lib/CodeGen/CGObjC.cpp clang/test/CodeGenObjCXX/arc-constexpr.mm Index: clang/test/CodeGenObjCXX/arc-constexpr.mm === --- clang/test/CodeGenObjCXX/arc-constexpr.mm +++ clang/test/CodeGenObjCXX/arc-constexpr.mm @@ -1,18 +1,51 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag +@class NSString; -// CHECK: define void @_Z5test1v +// CHECK-LABEL: define void @_Z5test1v // CHECK: %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]* // CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] // CHECK: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* // CHECK: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]] // CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]] // CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null) - -@class NSString; - void test1() { constexpr NSString *S = @"abc"; } + +// CHECK-LABEL: define void @_Z5test2v +// CHECK: %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]] +// CHECK: %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]] +// CHECK: %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null) +// CHECK: %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null) +void test2() { + constexpr NSString *Const = @"abc"; + // In IR RefConst should be initialized with Const initializer instead of + // reading from variable. + NSString* RefConst = Const; +} + +// CHECK-LABEL: define void @_Z5test3v +// CHECK: %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK: store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null) +// CHECK: %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]]) +void test3() { + __weak constexpr NSString *WeakConst = @"abc"; + NSString* RefWeakConst = WeakConst; +} Index: clang/lib/CodeGen/CGObjC.cpp === --- clang/lib/CodeGen/CGObjC.cpp +++ clang/lib/CodeGen/CGObjC.cpp @@ -2446,26 +2446,35 @@ EHStack.pushCleanup(NormalCleanup, Ptr); } -static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, - LValue lvalue, - QualType type) { - switch (type.getObjCLifetime()) { +static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime lifetime) { + switch (lifetime) { case Qualifiers::OCL_None: case Qualifiers::OCL_ExplicitNone: case Qualifiers::OCL_Strong: case Qualifiers::OCL_Autoreleasing: -return TryEmitResult(CGF.EmitLoadOfLValue(lvalue, - SourceLocation()).getScalarVal(), - false); +return true; case Qualifiers::OCL_Weak: -return TryEmitResult(CGF.EmitARCLoadWeakRetained(lvalue.getAddress()), - true); +return false; } llvm_unreachable("impossible lifetime!"); } +static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, + LValue lvalue, + QualType type) { + llvm::Value *result; + bool shouldRetain = shouldRetainObjCLifetime(type.getObjCLifetime()); + if (shouldRetain) { +result = CGF.EmitLoadOfLValue(lvalue, SourceLocation()).getScalarVal(); + } else { +assert(type.getObjCLifetime()
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai updated this revision to Diff 172049. vsapsai added a comment. - Rename `EmitConstant` to `emitScalarConstant`. - Tweak comment to be explicitly about intended IR code, not about Obj-C++ code. https://reviews.llvm.org/D53674 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/CGObjC.cpp clang/lib/CodeGen/CodeGenFunction.h clang/test/CodeGenObjCXX/arc-constexpr.mm Index: clang/test/CodeGenObjCXX/arc-constexpr.mm === --- clang/test/CodeGenObjCXX/arc-constexpr.mm +++ clang/test/CodeGenObjCXX/arc-constexpr.mm @@ -1,18 +1,51 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag +@class NSString; -// CHECK: define void @_Z5test1v +// CHECK-LABEL: define void @_Z5test1v // CHECK: %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]* // CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] // CHECK: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* // CHECK: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]] // CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]] // CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null) - -@class NSString; - void test1() { constexpr NSString *S = @"abc"; } + +// CHECK-LABEL: define void @_Z5test2v +// CHECK: %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]] +// CHECK: %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]] +// CHECK: %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null) +// CHECK: %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null) +void test2() { + constexpr NSString *Const = @"abc"; + // In IR RefConst should be initialized with Const initializer instead of + // reading from variable. + NSString* RefConst = Const; +} + +// CHECK-LABEL: define void @_Z5test3v +// CHECK: %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK: store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null) +// CHECK: %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]]) +void test3() { + __weak constexpr NSString *WeakConst = @"abc"; + NSString* RefWeakConst = WeakConst; +} Index: clang/lib/CodeGen/CodeGenFunction.h === --- clang/lib/CodeGen/CodeGenFunction.h +++ clang/lib/CodeGen/CodeGenFunction.h @@ -3524,6 +3524,7 @@ ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr); ConstantEmission tryEmitAsConstant(const MemberExpr *ME); + llvm::Value *emitScalarConstant(const ConstantEmission &Constant, Expr *E); RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e, AggValueSlot slot = AggValueSlot::ignored()); Index: clang/lib/CodeGen/CGObjC.cpp === --- clang/lib/CodeGen/CGObjC.cpp +++ clang/lib/CodeGen/CGObjC.cpp @@ -2446,26 +2446,35 @@ EHStack.pushCleanup(NormalCleanup, Ptr); } -static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, - LValue lvalue, - QualType type) { - switch (type.getObjCLifetime()) { +static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime lifetime) { + switch (lifetime) { case Qualifiers::OCL_None: case Qualifiers::OCL_ExplicitNone: case Qualifiers::OCL_Strong: case Qualifiers::OCL_Autoreleasing: -return TryEmitResult(CGF.EmitLoadOfLValue(lvalue, - SourceL
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:2480 + SuppressResultRetain); } vsapsai wrote: > rjmccall wrote: > > This switch is just checking what you already computed as > > `SuppressResultRetain`. Please just assert in the second case that the > > qualifier is `OCL_Weak`. > > > > Also, please stay consistent with the surrounding capitalization of local > > variables. > Not entirely sure I understand the comment about switch correctly. Will > change the code according to my understanding. That's exactly what I was asking for, thank you. https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:2527 + return TryEmitResult(CGF.EmitScalarExpr(e), + !shouldRetainObjCLifetime(type.getObjCLifetime())); + } vsapsai wrote: > rjmccall wrote: > > Can we test constant-evaluability directly instead of only applying this > > when the declaration isn't otherwise used? > We can use `IsVariableAConstantExpression` but it requires removing const > from `Decl`. > > Another way to test constant-evaluability is `tryEmitAsConstant`. It still > requires removing const qualifier but overall it is a good approach. It > captures the intention well and implementation-wise it is what > `CGF.EmitScalarExpr` is doing anyway. I'll try it and will show how it looks > like. If we go with `tryEmitAsConstant`, there is a separate review D53725 to use `EmitConstant` in more places. https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai updated this revision to Diff 171168. vsapsai added a comment. - Address review comments. https://reviews.llvm.org/D53674 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/CGObjC.cpp clang/lib/CodeGen/CodeGenFunction.h clang/test/CodeGenObjCXX/arc-constexpr.mm Index: clang/test/CodeGenObjCXX/arc-constexpr.mm === --- clang/test/CodeGenObjCXX/arc-constexpr.mm +++ clang/test/CodeGenObjCXX/arc-constexpr.mm @@ -1,18 +1,50 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag +@class NSString; -// CHECK: define void @_Z5test1v +// CHECK-LABEL: define void @_Z5test1v // CHECK: %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]* // CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] // CHECK: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* // CHECK: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]] // CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]] // CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null) - -@class NSString; - void test1() { constexpr NSString *S = @"abc"; } + +// CHECK-LABEL: define void @_Z5test2v +// CHECK: %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]] +// CHECK: %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]] +// CHECK: %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null) +// CHECK: %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null) +void test2() { + constexpr NSString *Const = @"abc"; + // Initialize RefConst with Const initializer instead of reading from variable. + NSString* RefConst = Const; +} + +// CHECK-LABEL: define void @_Z5test3v +// CHECK: %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK: store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null) +// CHECK: %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]]) +void test3() { + __weak constexpr NSString *WeakConst = @"abc"; + NSString* RefWeakConst = WeakConst; +} Index: clang/lib/CodeGen/CodeGenFunction.h === --- clang/lib/CodeGen/CodeGenFunction.h +++ clang/lib/CodeGen/CodeGenFunction.h @@ -3524,6 +3524,7 @@ ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr); ConstantEmission tryEmitAsConstant(const MemberExpr *ME); + llvm::Value *EmitConstant(const ConstantEmission &Constant, Expr *E); RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e, AggValueSlot slot = AggValueSlot::ignored()); Index: clang/lib/CodeGen/CGObjC.cpp === --- clang/lib/CodeGen/CGObjC.cpp +++ clang/lib/CodeGen/CGObjC.cpp @@ -2446,26 +2446,35 @@ EHStack.pushCleanup(NormalCleanup, Ptr); } -static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, - LValue lvalue, - QualType type) { - switch (type.getObjCLifetime()) { +static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime lifetime) { + switch (lifetime) { case Qualifiers::OCL_None: case Qualifiers::OCL_ExplicitNone: case Qualifiers::OCL_Strong: case Qualifiers::OCL_Autoreleasing: -return TryEmitResult(CGF.EmitLoadOfLValue(lvalue, - SourceLocation()).getScalarVal(), - false); +return true; case Qualifiers::OCL_Weak: -return TryEmitRe
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:2480 + SuppressResultRetain); } rjmccall wrote: > This switch is just checking what you already computed as > `SuppressResultRetain`. Please just assert in the second case that the > qualifier is `OCL_Weak`. > > Also, please stay consistent with the surrounding capitalization of local > variables. Not entirely sure I understand the comment about switch correctly. Will change the code according to my understanding. Comment at: clang/lib/CodeGen/CGObjC.cpp:2527 + return TryEmitResult(CGF.EmitScalarExpr(e), + !shouldRetainObjCLifetime(type.getObjCLifetime())); + } rjmccall wrote: > Can we test constant-evaluability directly instead of only applying this when > the declaration isn't otherwise used? We can use `IsVariableAConstantExpression` but it requires removing const from `Decl`. Another way to test constant-evaluability is `tryEmitAsConstant`. It still requires removing const qualifier but overall it is a good approach. It captures the intention well and implementation-wise it is what `CGF.EmitScalarExpr` is doing anyway. I'll try it and will show how it looks like. https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:2480 + SuppressResultRetain); } This switch is just checking what you already computed as `SuppressResultRetain`. Please just assert in the second case that the qualifier is `OCL_Weak`. Also, please stay consistent with the surrounding capitalization of local variables. Comment at: clang/lib/CodeGen/CGObjC.cpp:2527 + return TryEmitResult(CGF.EmitScalarExpr(e), + !shouldRetainObjCLifetime(type.getObjCLifetime())); + } Can we test constant-evaluability directly instead of only applying this when the declaration isn't otherwise used? https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.
vsapsai created this revision. vsapsai added reviewers: ahatanak, rjmccall. Herald added a subscriber: dexonsmith. Failed assertion is > Assertion failed: ((ND->isUsed(false) || !isa(ND) || > !E->getLocation().isValid()) && "Should not use decl without marking it > used!"), function EmitDeclRefLValue, file > llvm-project/clang/lib/CodeGen/CGExpr.cpp, line 2437. `EmitDeclRefLValue` mentions > // A DeclRefExpr for a reference initialized by a constant expression can > // appear without being odr-used. Directly emit the constant initializer. The fix is in using the similar approach for non-references of non-odr-used variables. `EmitScalarExpr` will try to emit constant if possible and we can use resulting `llvm::Value *` without performing `EmitLValue`. rdar://problem/40650504 https://reviews.llvm.org/D53674 Files: clang/lib/CodeGen/CGObjC.cpp clang/test/CodeGenObjCXX/arc-constexpr.mm Index: clang/test/CodeGenObjCXX/arc-constexpr.mm === --- clang/test/CodeGenObjCXX/arc-constexpr.mm +++ clang/test/CodeGenObjCXX/arc-constexpr.mm @@ -1,18 +1,50 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag +@class NSString; -// CHECK: define void @_Z5test1v +// CHECK-LABEL: define void @_Z5test1v // CHECK: %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]* // CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] // CHECK: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* // CHECK: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]] // CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]] // CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null) - -@class NSString; - void test1() { constexpr NSString *S = @"abc"; } + +// CHECK-LABEL: define void @_Z5test2v +// CHECK: %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]] +// CHECK: %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]* +// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]] +// CHECK: %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null) +// CHECK: %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null) +void test2() { + constexpr NSString *Const = @"abc"; + // Initialize RefConst with Const initializer instead of reading from variable. + NSString* RefConst = Const; +} + +// CHECK-LABEL: define void @_Z5test3v +// CHECK: %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]* +// CHECK: %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] +// CHECK: store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK: %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]] +// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null) +// CHECK: %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]] +// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]]) +void test3() { + __weak constexpr NSString *WeakConst = @"abc"; + NSString* RefWeakConst = WeakConst; +} Index: clang/lib/CodeGen/CGObjC.cpp === --- clang/lib/CodeGen/CGObjC.cpp +++ clang/lib/CodeGen/CGObjC.cpp @@ -2446,21 +2446,37 @@ EHStack.pushCleanup(NormalCleanup, Ptr); } +static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime Lifetime) { + switch (Lifetime) { + case Qualifiers::OCL_None: + case Qualifiers::OCL_ExplicitNone: + case Qualifiers::OCL_Strong: + case Qualifiers::OCL_Autoreleasing: +return true; + + case Qualifiers::OCL_Weak: +return false; + } + + llvm_unreachable("impossible lifetime!"); +} + static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF, LValue lvalue, QualType type) { + bool SuppressResultRetain = !shouldRetainObjCLifetime(type.getObjCLifetime()); switch (type.getObjCLifetime