Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL270808: [ObjC] Remove _Atomic from return type and parameter 
type of (authored by ahatanak).

Changed prior to commit:
  http://reviews.llvm.org/D20407?vs=58544=58549#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20407

Files:
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/Sema/SemaObjCProperty.cpp
  cfe/trunk/test/CodeGenObjC/property-atomic-bool.m
  cfe/trunk/test/SemaObjC/property-atomic-bool.m

Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -1080,6 +1080,9 @@
   /// Strip Objective-C "__kindof" types from the given type.
   QualType stripObjCKindOfType(const ASTContext ) const;
 
+  /// Remove all qualifiers including _Atomic.
+  QualType getAtomicUnqualifiedType() const;
+
 private:
   // These methods are implemented in a separate translation unit;
   // "static"-ize them to avoid creating temporary QualTypes in the
Index: cfe/trunk/test/SemaObjC/property-atomic-bool.m
===
--- cfe/trunk/test/SemaObjC/property-atomic-bool.m
+++ cfe/trunk/test/SemaObjC/property-atomic-bool.m
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-dump "%s" 2>&1 | FileCheck %s
+
+// CHECK: TypedefDecl {{.*}} referenced AtomicBool '_Atomic(_Bool)'
+// CHECK:  AtomicType {{.*}} '_Atomic(_Bool)'
+// CHECK:   BuiltinType {{.*}} '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A0
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A1
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A2
+// CHECK:  ObjCIvarDecl {{.*}} p '_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A3
+// CHECK:  ObjCIvarDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+
+typedef _Atomic(_Bool) AtomicBool;
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1
+@property(nonatomic) AtomicBool p;
+@end
+@implementation A1
+@end
+
+@interface A2 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+
+@implementation A2
+@synthesize p;
+@end
+
+@interface A3 {
+  AtomicBool p;
+}
+@property AtomicBool p;
+@end
+
+@implementation A3
+@synthesize p;
+@end
Index: cfe/trunk/test/CodeGenObjC/property-atomic-bool.m
===
--- cfe/trunk/test/CodeGenObjC/property-atomic-bool.m
+++ cfe/trunk/test/CodeGenObjC/property-atomic-bool.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -emit-llvm -x objective-c %s -o - | FileCheck %s
+
+// CHECK: define internal zeroext i1 @"\01-[A0 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} seq_cst
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %[[ATOMIC_LOAD]] to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A0 setP:]"({{.*}} i1 zeroext {{.*}})
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst
+// CHECK:   ret void
+
+// CHECK: define internal zeroext i1 @"\01-[A1 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} unordered
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %load to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A1 setP:]"({{.*}} i1 zeroext %p)
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} unordered
+// CHECK:   ret void
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+@implementation A1
+@synthesize p;
+@end
Index: cfe/trunk/lib/CodeGen/CGObjC.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjC.cpp
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp
@@ -897,9 +897,8 @@
 
 // Currently, all atomic accesses have to be through integer
 // types, so there's no point in trying to pick a prettier type.
-llvm::Type *bitcastType =
-  llvm::Type::getIntNTy(getLLVMContext(),
-  

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak updated this revision to Diff 58544.
ahatanak added a comment.

Thanks for the review. I've removed parameter ASTContext that was unused.


http://reviews.llvm.org/D20407

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaObjCProperty.cpp
  test/CodeGenObjC/property-atomic-bool.m
  test/SemaObjC/property-atomic-bool.m

Index: test/SemaObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/SemaObjC/property-atomic-bool.m
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-dump "%s" 2>&1 | FileCheck %s
+
+// CHECK: TypedefDecl {{.*}} referenced AtomicBool '_Atomic(_Bool)'
+// CHECK:  AtomicType {{.*}} '_Atomic(_Bool)'
+// CHECK:   BuiltinType {{.*}} '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A0
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A1
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A2
+// CHECK:  ObjCIvarDecl {{.*}} p '_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A3
+// CHECK:  ObjCIvarDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+
+typedef _Atomic(_Bool) AtomicBool;
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1
+@property(nonatomic) AtomicBool p;
+@end
+@implementation A1
+@end
+
+@interface A2 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+
+@implementation A2
+@synthesize p;
+@end
+
+@interface A3 {
+  AtomicBool p;
+}
+@property AtomicBool p;
+@end
+
+@implementation A3
+@synthesize p;
+@end
Index: test/CodeGenObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/CodeGenObjC/property-atomic-bool.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -emit-llvm -x objective-c %s -o - | FileCheck %s
+
+// CHECK: define internal zeroext i1 @"\01-[A0 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} seq_cst
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %[[ATOMIC_LOAD]] to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A0 setP:]"({{.*}} i1 zeroext {{.*}})
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst
+// CHECK:   ret void
+
+// CHECK: define internal zeroext i1 @"\01-[A1 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} unordered
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %load to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A1 setP:]"({{.*}} i1 zeroext %p)
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} unordered
+// CHECK:   ret void
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+@implementation A1
+@synthesize p;
+@end
Index: lib/Sema/SemaObjCProperty.cpp
===
--- lib/Sema/SemaObjCProperty.cpp
+++ lib/Sema/SemaObjCProperty.cpp
@@ -1493,24 +1493,26 @@
   if (!GetterMethod)
 return false;
   QualType GetterType = GetterMethod->getReturnType().getNonReferenceType();
-  QualType PropertyIvarType = property->getType().getNonReferenceType();
-  bool compat = Context.hasSameType(PropertyIvarType, GetterType);
+  QualType PropertyRValueType =
+  property->getType().getNonReferenceType().getAtomicUnqualifiedType();
+  bool compat = Context.hasSameType(PropertyRValueType, GetterType);
   if (!compat) {
 const ObjCObjectPointerType *propertyObjCPtr = nullptr;
 const ObjCObjectPointerType *getterObjCPtr = nullptr;
-if ((propertyObjCPtr = PropertyIvarType->getAs()) && 
+if ((propertyObjCPtr =
+ PropertyRValueType->getAs()) &&
 (getterObjCPtr = GetterType->getAs()))
   compat = Context.canAssignObjCInterfaces(getterObjCPtr, propertyObjCPtr);
-else if (CheckAssignmentConstraints(Loc, GetterType, PropertyIvarType) 
+else if (CheckAssignmentConstraints(Loc, GetterType, PropertyRValueType)
   != Compatible) {
   Diag(Loc, diag::error_property_accessor_type)
-<< 

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added a comment.

One minor improvement to the API and LGTM.



Comment at: include/clang/AST/Type.h:1084
@@ +1083,3 @@
+  /// Remove all qualifiers including _Atomic.
+  QualType getAtomicUnqualifiedType(const ASTContext ) const;
+

This doesn't need an ASTContext.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added a comment.

In http://reviews.llvm.org/D20407#440050, @ahatanak wrote:

> In http://reviews.llvm.org/D20407#439951, @rjmccall wrote:
>
> > The C standard is poorly-written in this area, but I think it would be 
> > reasonable for CheckFunctionReturnType to just silently remove _Atomic.  
> > (You will not be able to just re-use your new method there; removing other 
> > qualifiers is not acceptable, I'm afraid.)
>
>
> Do you mean I should fix those cases in this patch too?


No, sorry, just pointing something out for your other patch.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

In http://reviews.llvm.org/D20407#439951, @rjmccall wrote:

> The C standard is poorly-written in this area, but I think it would be 
> reasonable for CheckFunctionReturnType to just silently remove _Atomic.  (You 
> will not be able to just re-use your new method there; removing other 
> qualifiers is not acceptable, I'm afraid.)


Do you mean I should fix those cases in this patch too?


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak updated this revision to Diff 58512.
ahatanak added a comment.

Rename variables.


http://reviews.llvm.org/D20407

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaObjCProperty.cpp
  test/CodeGenObjC/property-atomic-bool.m
  test/SemaObjC/property-atomic-bool.m

Index: test/SemaObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/SemaObjC/property-atomic-bool.m
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-dump "%s" 2>&1 | FileCheck %s
+
+// CHECK: TypedefDecl {{.*}} referenced AtomicBool '_Atomic(_Bool)'
+// CHECK:  AtomicType {{.*}} '_Atomic(_Bool)'
+// CHECK:   BuiltinType {{.*}} '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A0
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A1
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A2
+// CHECK:  ObjCIvarDecl {{.*}} p '_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A3
+// CHECK:  ObjCIvarDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+
+typedef _Atomic(_Bool) AtomicBool;
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1
+@property(nonatomic) AtomicBool p;
+@end
+@implementation A1
+@end
+
+@interface A2 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+
+@implementation A2
+@synthesize p;
+@end
+
+@interface A3 {
+  AtomicBool p;
+}
+@property AtomicBool p;
+@end
+
+@implementation A3
+@synthesize p;
+@end
Index: test/CodeGenObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/CodeGenObjC/property-atomic-bool.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -emit-llvm -x objective-c %s -o - | FileCheck %s
+
+// CHECK: define internal zeroext i1 @"\01-[A0 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} seq_cst
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %[[ATOMIC_LOAD]] to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A0 setP:]"({{.*}} i1 zeroext {{.*}})
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst
+// CHECK:   ret void
+
+// CHECK: define internal zeroext i1 @"\01-[A1 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} unordered
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %load to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A1 setP:]"({{.*}} i1 zeroext %p)
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} unordered
+// CHECK:   ret void
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+@implementation A1
+@synthesize p;
+@end
Index: lib/Sema/SemaObjCProperty.cpp
===
--- lib/Sema/SemaObjCProperty.cpp
+++ lib/Sema/SemaObjCProperty.cpp
@@ -1493,24 +1493,27 @@
   if (!GetterMethod)
 return false;
   QualType GetterType = GetterMethod->getReturnType().getNonReferenceType();
-  QualType PropertyIvarType = property->getType().getNonReferenceType();
-  bool compat = Context.hasSameType(PropertyIvarType, GetterType);
+  QualType PropertyRValueType =
+  property->getType().getNonReferenceType().getAtomicUnqualifiedType(
+  Context);
+  bool compat = Context.hasSameType(PropertyRValueType, GetterType);
   if (!compat) {
 const ObjCObjectPointerType *propertyObjCPtr = nullptr;
 const ObjCObjectPointerType *getterObjCPtr = nullptr;
-if ((propertyObjCPtr = PropertyIvarType->getAs()) && 
+if ((propertyObjCPtr =
+ PropertyRValueType->getAs()) &&
 (getterObjCPtr = GetterType->getAs()))
   compat = Context.canAssignObjCInterfaces(getterObjCPtr, propertyObjCPtr);
-else if (CheckAssignmentConstraints(Loc, GetterType, PropertyIvarType) 
+else if (CheckAssignmentConstraints(Loc, GetterType, PropertyRValueType)
   != Compatible) {
   Diag(Loc, diag::error_property_accessor_type)
-<< property->getDeclName() << PropertyIvarType
+

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added a comment.

In http://reviews.llvm.org/D20407#439887, @ahatanak wrote:

> I reverted the changes I made in SemaDeclObjC.cpp as they weren't needed to 
> pass the regression tests I added. clang still asserts when it compiles an 
> objective-c method returning _Atomic and those changes will become necessary 
> when I fix that later. Normal functions returning _Atomic doesn't compile 
> either.


The C standard is poorly-written in this area, but I think it would be 
reasonable for CheckFunctionReturnType to just silently remove _Atomic.  (You 
will not be able to just re-use your new method there; removing other 
qualifiers is not acceptable, I'm afraid.)


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added inline comments.


Comment at: lib/AST/Type.cpp:1282
@@ -1277,1 +1281,3 @@
+}
+
 Optional Type::getObjCSubstitutions(

ahatanak wrote:
> I added getTypePtr() because the code didn't compile.
Sure.


Comment at: lib/CodeGen/CGObjC.cpp:901
@@ -903,1 +900,3 @@
+uint64_t ivarSize = getContext().toBits(strategy.getIvarSize());
+llvm::Type *bitcastType = llvm::Type::getIntNTy(getLLVMContext(), 
ivarSize);
 bitcastType = bitcastType->getPointerTo(); // addrspace 0 okay

ahatanak wrote:
> I've only changed ivarSize. I can change the variable names defined below 
> (RetTy and RetTySize) too if that is necesasry.
Please do.


Comment at: lib/Sema/SemaObjCProperty.cpp:1497
@@ -1496,2 +1496,3 @@
   QualType PropertyIvarType = property->getType().getNonReferenceType();
-  bool compat = Context.hasSameType(PropertyIvarType, GetterType);
+  QualType AtomicUnquailifiedType =
+  PropertyIvarType.getAtomicUnqualifiedType(Context);

You know what, I'm sorry, I didn't look closely enough at the context.  Please 
just rename PropertyIvarType to something like PropertyRValueType.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak marked 2 inline comments as done.
ahatanak added a comment.

I reverted the changes I made in SemaDeclObjC.cpp as they weren't needed to 
pass the regression tests I added. clang still asserts when it compiles an 
objective-c method returning _Atomic and those changes will become necessary 
when I fix that later. Normal functions returning _Atomic doesn't compile 
either.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak marked 7 inline comments as done.


Comment at: lib/AST/Type.cpp:1282
@@ -1277,1 +1281,3 @@
+}
+
 Optional Type::getObjCSubstitutions(

I added getTypePtr() because the code didn't compile.


Comment at: lib/CodeGen/CGObjC.cpp:901
@@ -903,1 +900,3 @@
+uint64_t ivarSize = getContext().toBits(strategy.getIvarSize());
+llvm::Type *bitcastType = llvm::Type::getIntNTy(getLLVMContext(), 
ivarSize);
 bitcastType = bitcastType->getPointerTo(); // addrspace 0 okay

I've only changed ivarSize. I can change the variable names defined below 
(RetTy and RetTySize) too if that is necesasry.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak updated this revision to Diff 58495.
ahatanak added a comment.

Address John's review comments.


http://reviews.llvm.org/D20407

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaObjCProperty.cpp
  test/CodeGenObjC/property-atomic-bool.m
  test/SemaObjC/property-atomic-bool.m

Index: test/SemaObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/SemaObjC/property-atomic-bool.m
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-dump "%s" 2>&1 | FileCheck %s
+
+// CHECK: TypedefDecl {{.*}} referenced AtomicBool '_Atomic(_Bool)'
+// CHECK:  AtomicType {{.*}} '_Atomic(_Bool)'
+// CHECK:   BuiltinType {{.*}} '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A0
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A1
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A2
+// CHECK:  ObjCIvarDecl {{.*}} p '_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A3
+// CHECK:  ObjCIvarDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+
+typedef _Atomic(_Bool) AtomicBool;
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1
+@property(nonatomic) AtomicBool p;
+@end
+@implementation A1
+@end
+
+@interface A2 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+
+@implementation A2
+@synthesize p;
+@end
+
+@interface A3 {
+  AtomicBool p;
+}
+@property AtomicBool p;
+@end
+
+@implementation A3
+@synthesize p;
+@end
Index: test/CodeGenObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/CodeGenObjC/property-atomic-bool.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -emit-llvm -x objective-c %s -o - | FileCheck %s
+
+// CHECK: define internal zeroext i1 @"\01-[A0 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} seq_cst
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %[[ATOMIC_LOAD]] to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A0 setP:]"({{.*}} i1 zeroext {{.*}})
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst
+// CHECK:   ret void
+
+// CHECK: define internal zeroext i1 @"\01-[A1 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} unordered
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %load to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A1 setP:]"({{.*}} i1 zeroext %p)
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} unordered
+// CHECK:   ret void
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+@implementation A1
+@synthesize p;
+@end
Index: lib/Sema/SemaObjCProperty.cpp
===
--- lib/Sema/SemaObjCProperty.cpp
+++ lib/Sema/SemaObjCProperty.cpp
@@ -1494,23 +1494,26 @@
 return false;
   QualType GetterType = GetterMethod->getReturnType().getNonReferenceType();
   QualType PropertyIvarType = property->getType().getNonReferenceType();
-  bool compat = Context.hasSameType(PropertyIvarType, GetterType);
+  QualType AtomicUnquailifiedType =
+  PropertyIvarType.getAtomicUnqualifiedType(Context);
+  bool compat = Context.hasSameType(AtomicUnquailifiedType, GetterType);
   if (!compat) {
 const ObjCObjectPointerType *propertyObjCPtr = nullptr;
 const ObjCObjectPointerType *getterObjCPtr = nullptr;
-if ((propertyObjCPtr = PropertyIvarType->getAs()) && 
+if ((propertyObjCPtr =
+ AtomicUnquailifiedType->getAs()) &&
 (getterObjCPtr = GetterType->getAs()))
   compat = Context.canAssignObjCInterfaces(getterObjCPtr, propertyObjCPtr);
-else if (CheckAssignmentConstraints(Loc, GetterType, PropertyIvarType) 
+else if (CheckAssignmentConstraints(Loc, GetterType, AtomicUnquailifiedType)
   != Compatible) {
   Diag(Loc, diag::error_property_accessor_type)
 << property->getDeclName() << PropertyIvarType
 << 

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added inline comments.


Comment at: include/clang/AST/Type.h:1084
@@ +1083,3 @@
+  /// Strip typedefs and atomic from the given type.
+  QualType getDesugaredAtomicValueType(const ASTContext ) const;
+

Please name this getAtomicUnqualifiedType() and have it promise to remove all 
qualifiers including _Atomic.  It should not promise to remove type sugar.


Comment at: lib/AST/Type.cpp:1282
@@ +1281,3 @@
+  return *this;
+}
+

This should just be:
  if (auto AT = getAs()) {
return AT->getValueType().getUnqualifiedType();
  } else {
return getUnqualifiedType();
  }


Comment at: lib/CodeGen/CGObjC.cpp:901
@@ -903,1 +900,3 @@
+uint64_t IVarSize = getContext().toBits(strategy.getIvarSize());
+llvm::Type *bitcastType = llvm::Type::getIntNTy(getLLVMContext(), 
IVarSize);
 bitcastType = bitcastType->getPointerTo(); // addrspace 0 okay

The code around here uses lowercase identifiers, please be consistent.


Comment at: lib/CodeGen/CGObjC.cpp:1023
@@ -1014,1 +1022,3 @@
+value,
+ConvertType(propType.getDesugaredAtomicValueType(getContext(;
 value = Builder.CreateBitCast(

I don't understand the purpose of doing two bitcasts here, so let's just drop 
this first one.


Comment at: lib/Sema/SemaDeclObjC.cpp:4319
@@ +4318,3 @@
+  Context, MethodLoc, EndLoc, Sel,
+  resultDeclType.getDesugaredAtomicValueType(Context),
+  ReturnTInfo, CurContext, MethodType == tok::minus, isVariadic,

Hmm.  I'm not sure you should be doing this here; or at least, if we're going 
to do it, we should probably be doing it consistently in 
CheckFunctionReturnType.


Comment at: lib/Sema/SemaDeclObjC.cpp:4361
@@ -4361,1 +4360,3 @@
+ObjCMethod, StartLoc, ArgInfo[i].NameLoc, ArgInfo[i].Name,
+ArgType.getDesugaredAtomicValueType(Context), DI, SC_None);
 

Same thing.  If the user wants to explicitly declare a parameter _Atomic, they 
can do so.


Comment at: lib/Sema/SemaObjCProperty.cpp:1497
@@ -1496,2 +1496,3 @@
   QualType PropertyIvarType = property->getType().getNonReferenceType();
+  PropertyIvarType = PropertyIvarType.getDesugaredAtomicValueType(Context);
   bool compat = Context.hasSameType(PropertyIvarType, GetterType);

This is incorrect.  The property ivar type should definitely remain _Atomic if 
the property was declared that way.


Comment at: lib/Sema/SemaObjCProperty.cpp:2210
@@ -2209,1 +2209,3 @@
+QualType resultTy =
+property->getType().getDesugaredAtomicValueType(Context);
 if (property->getPropertyAttributes() &

Please pull this above the comment and give it its own comment, something like:
  // The getter returns the declared property type with all qualifiers removed.


Comment at: lib/Sema/SemaObjCProperty.cpp:2282
@@ -2279,2 +2281,3 @@
   QualType paramTy = property->getType().getUnqualifiedType();
+  paramTy = paramTy.getDesugaredAtomicValueType(Context);
   if (property->getPropertyAttributes() &

Same thing: please pull this above and give it its own comment.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak updated this revision to Diff 58144.
ahatanak added a comment.

Rewrote the patch based on John's review comment.

Remove typedefs and _Atomic from the return and parameter types of getters and 
setters of objective-c properties.


http://reviews.llvm.org/D20407

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaObjCProperty.cpp
  test/CodeGenObjC/property-atomic-bool.m
  test/SemaObjC/property-atomic-bool.m

Index: test/SemaObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/SemaObjC/property-atomic-bool.m
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-dump "%s" 2>&1 | FileCheck %s
+
+// CHECK: TypedefDecl {{.*}} referenced AtomicBool '_Atomic(_Bool)'
+// CHECK:  AtomicType {{.*}} '_Atomic(_Bool)'
+// CHECK:   BuiltinType {{.*}} '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A0
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A1
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' {{.*}} nonatomic
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A2
+// CHECK:  ObjCIvarDecl {{.*}} p '_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p '_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+// CHECK: ObjCInterfaceDecl {{.*}} A3
+// CHECK:  ObjCIvarDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)' protected
+// CHECK:  ObjCPropertyDecl {{.*}} p 'AtomicBool':'_Atomic(_Bool)'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - p '_Bool'
+// CHECK:  ObjCMethodDecl {{.*}} implicit - setP: 'void'
+// CHECK:   ParmVarDecl {{.*}} p '_Bool'
+
+typedef _Atomic(_Bool) AtomicBool;
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1
+@property(nonatomic) AtomicBool p;
+@end
+@implementation A1
+@end
+
+@interface A2 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+
+@implementation A2
+@synthesize p;
+@end
+
+@interface A3 {
+  AtomicBool p;
+}
+@property AtomicBool p;
+@end
+
+@implementation A3
+@synthesize p;
+@end
Index: test/CodeGenObjC/property-atomic-bool.m
===
--- /dev/null
+++ test/CodeGenObjC/property-atomic-bool.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -emit-llvm -x objective-c %s -o - | FileCheck %s
+
+// CHECK: define internal zeroext i1 @"\01-[A0 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} seq_cst
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %[[ATOMIC_LOAD]] to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A0 setP:]"({{.*}} i1 zeroext {{.*}})
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst
+// CHECK:   ret void
+
+// CHECK: define internal zeroext i1 @"\01-[A1 p]"(
+// CHECK:   %[[ATOMIC_LOAD:.*]] = load atomic i8, i8* %{{.*}} unordered
+// CHECK:   %[[TOBOOL:.*]] = trunc i8 %load to i1
+// CHECK:   ret i1 %[[TOBOOL]]
+
+// CHECK: define internal void @"\01-[A1 setP:]"({{.*}} i1 zeroext %p)
+// CHECK:   store atomic i8 %{{.*}}, i8* %{{.*}} unordered
+// CHECK:   ret void
+
+@interface A0
+@property(nonatomic) _Atomic(_Bool) p;
+@end
+@implementation A0
+@end
+
+@interface A1 {
+  _Atomic(_Bool) p;
+}
+@property _Atomic(_Bool) p;
+@end
+@implementation A1
+@synthesize p;
+@end
Index: lib/Sema/SemaObjCProperty.cpp
===
--- lib/Sema/SemaObjCProperty.cpp
+++ lib/Sema/SemaObjCProperty.cpp
@@ -1494,6 +1494,7 @@
 return false;
   QualType GetterType = GetterMethod->getReturnType().getNonReferenceType();
   QualType PropertyIvarType = property->getType().getNonReferenceType();
+  PropertyIvarType = PropertyIvarType.getDesugaredAtomicValueType(Context);
   bool compat = Context.hasSameType(PropertyIvarType, GetterType);
   if (!compat) {
 const ObjCObjectPointerType *propertyObjCPtr = nullptr;
@@ -2205,7 +2206,8 @@
 SourceLocation Loc = property->getLocation();
 
 // If the property is null_resettable, the getter returns nonnull.
-QualType resultTy = property->getType();
+QualType resultTy =
+property->getType().getDesugaredAtomicValueType(Context);
 if (property->getPropertyAttributes() &
 ObjCPropertyDecl::OBJC_PR_null_resettable) {
   QualType modifiedTy = resultTy;
@@ -2277,6 +2279,7 @@
   // If the property is null_resettable, the setter accepts a
   // nullable value.
   QualType paramTy = 

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added a comment.

Top-level attributes shouldn't really be affecting the ABI or code-generation 
in any way, so desugaring should be fine.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

I've managed to remove _Atomic from the types of the return and parameter of 
getter and setter functions in my new patch.

I'm not sure how I should handle typedefs though. If I had the following 
typedef,

typedef _Atomic(_Bool) AtomicBool,

would it be OK to desugar the typedef and then remove _Atomic? I'm not sure 
this is always correct because dusugaring typedefs will remove the attributes 
attached to them that might be important too.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

In http://reviews.llvm.org/D20407#433915, @rjmccall wrote:

> _Atomic is functionally a type qualifier and should be removed in Sema when 
> computing the result type of the getter and the parameter type of the setter. 
>  That is, if the user declares a property of type _Atomic(_Bool), we should 
> pretend that the property has type _Bool when creating the getter and setter.


That sounds like a more principled way to fix the problem. We'll have to remove 
_Atomic from return types of normal functions too, not just objective-c getters 
and setters, because otherwise programs like this won't compile:

  _Atomic(_Bool) foo1() {
A *a = [A new];
return a.p;
  }


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[John McCall via cfe-commits]

rjmccall added a comment.

_Atomic is functionally a type qualifier and should be removed in Sema when 
computing the result type of the getter and the parameter type of the setter.  
That is, if the user declares a property of type _Atomic(_Bool), we should 
pretend that the property has type _Bool when creating the getter and setter.


http://reviews.llvm.org/D20407



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

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

Also, it seems that there are bugs in the way clang handles functions whose 
return types are atomic. Clang asserts when compiling the following test case:

$ cat t1.c

  _Atomic _Bool b1;
  
  _Atomic _Bool foo1() {
return b1;
  }

$ clang -std=c11 -o - -S t1.c -emit-llvm


http://reviews.llvm.org/D20407



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