[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

2018-11-01 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-11-01 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-10-31 Thread John McCall via Phabricator via cfe-commits
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.

2018-10-31 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-10-31 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-10-26 Thread John McCall via Phabricator via cfe-commits
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.

2018-10-25 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-10-25 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-10-25 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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.

2018-10-24 Thread John McCall via Phabricator via cfe-commits
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.

2018-10-24 Thread Volodymyr Sapsai via Phabricator via cfe-commits
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