[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366063: [OpenCL] Deduce addr space for pointee of dependent 
types in instantiation. (authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64400?vs=209205=209831#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64400

Files:
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl


Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -4536,6 +4536,14 @@
   return Result;
 }
 
+/// Helper to deduce addr space of a pointee type in OpenCL mode.
+/// If the type is updated it will be overwritten in PointeeType param.
+static void deduceOpenCLPointeeAddrSpace(Sema , QualType ) 
{
+  if (PointeeType.getAddressSpace() == LangAS::Default)
+PointeeType = SemaRef.Context.getAddrSpaceQualType(PointeeType,
+   LangAS::opencl_generic);
+}
+
 template
 QualType TreeTransform::TransformPointerType(TypeLocBuilder ,
   PointerTypeLoc TL) {
@@ -4544,6 +4552,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (PointeeType->getAs()) {
 // A dependent pointer type 'T *' has is being transformed such
@@ -4582,6 +4593,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != TL.getPointeeLoc().getType()) {
@@ -4611,6 +4625,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != T->getPointeeTypeAsWritten()) {
Index: cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
===
--- cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
+++ cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
@@ -24,3 +24,42 @@
   alias_c1_ptr ptr = 
 };
 
+
+// Addr spaces for pointee of dependent types are not deduced
+// during parsing but during template instantiation instead.
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= 'x1 &(const x1 &) __generic'
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic 
x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (x1 *) __generic'
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1 *xx, __global x2 *bar) {
+  bar->foo(xx);
+}
+
+template 
+class x3 : public T {
+public:
+  //CHECK: -CXXConstructorDecl {{.*}} x3 'void (const x3 &) __generic'
+  x3(const x3 );
+};
+//CHECK: -CXXConstructorDecl {{.*}} x3 'void (const x3 &) __generic'
+template 
+x3::x3(const x3 ) {}


Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -4536,6 +4536,14 @@
   return Result;
 }
 
+/// Helper to deduce addr space of a pointee type in OpenCL mode.
+/// If the type is updated it will be overwritten in PointeeType param.
+static void deduceOpenCLPointeeAddrSpace(Sema , QualType ) {
+  if (PointeeType.getAddressSpace() == LangAS::Default)
+PointeeType = SemaRef.Context.getAddrSpaceQualType(PointeeType,
+   LangAS::opencl_generic);
+}
+
 template
 QualType TreeTransform::TransformPointerType(TypeLocBuilder ,
   PointerTypeLoc TL) {
@@ -4544,6 +4552,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (PointeeType->getAs()) {
 // A dependent pointer type 'T *' has is being transformed such
@@ -4582,6 +4593,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != TL.getPointeeLoc().getType()) {
@@ -4611,6 +4625,9 

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay.


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D64400#1582142 , @rjmccall wrote:

> There are some code paths that I think are common between the parser and 
> template instantiation, like `BuildPointerType` and `BuildReferenceType`, but 
> if you want to do context-sensitive inference that might not be good enough.


Actually for pointee types we don't need context-sensitive inference. This is 
mainly for regular types, but in templates we have much less use cases and I 
haven't caught any issue with the current implementation although we still keep 
some inference logic for dependent types. But it's simple enough. Potentially 
we can assess the corner cases as we go along and discover them.


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

There are some code paths that I think are common between the parser and 
template instantiation, like `BuildPointerType` and `BuildReferenceType`, but 
if you want to do context-sensitive inference that might not be good enough.


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: lib/Sema/SemaType.cpp:7421
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() && (!T->isPointerType() && !T->isReferenceType() &&

rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > I don't understand what you're saying here.  Why does inference depend on 
> > > whether the type is a template specialization?  And what does this have 
> > > to do with template arguments?  Also, address spaces in template 
> > > arguments are definitely part of the template argument and affect which 
> > > specialization you're naming.
> > What I am trying to say here is that an address space of a template 
> > argument isn't used as an address space of a template specialization and 
> > therefore we can deduce the address space of a template specialization 
> > since it's not going to be provided during the template instantiation.
> > 
> > Let's say we have specialization `MyClass`. The address space of `T` has 
> > nothing to do with the address space of `MyClass`. They are different. 
> > Therefore if the address space of `MyClass` is not provided explicitly 
> > it is ok to deduce it.
> > 
> > Does it make sense?
> Of course the address space of `T` has nothing to do with the address space 
> of `MyClass`, but that's true of literally every type, and you don't need 
> to add special cases checking for specific type spellings.
> 
> Why don't you just never infer address spaces on dependent types and then 
> infer them as necessary during instantiation?  Why is it important to infer 
> address spaces on any dependent type in the template pattern?
I quite like to keep the inference logic in one place mainly to avoid code 
duplication and simplify the architecture. However, it seems to be just much 
simpler to move the inference of pointee type into template instantiation.

I am thinking about the other cases i.e. non-pointer types and it seems that 
might be much harder to move there because the place we are transforming the 
type doesn't have information of where and how the type is being used. So in 
the future we might end up with inference logic scattered around the 
`TreeTransform` code propagated through `Subst*Type` calls rather than being 
kept in one place like we do here in SemaType. Not sure if you have better 
ideas how we could keep similar architecture in template instantiation too.


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 209205.
Anastasia added a comment.

- Moved addr space inference of pointee type into template instantiation.


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

https://reviews.llvm.org/D64400

Files:
  lib/Sema/TreeTransform.h
  test/SemaOpenCLCXX/address-space-deduction.cl


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- test/SemaOpenCLCXX/address-space-deduction.cl
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -38,3 +38,43 @@
   int foo[10];
   xxx([0]);
 }
+
+// Deducing addr spaces for template specialization is fine
+// addr space of template arg can't affecting the addr space
+// of specialization
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= 'x1 &(const x1 &) __generic'
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic 
x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (x1 *) __generic'
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1 *xx, __global x2 *bar) {
+  bar->foo(xx);
+}
+
+template 
+class x3 : public T {
+public:
+  //CHECK: -CXXConstructorDecl {{.*}} x3 'void (const x3 &) __generic'
+  x3(const x3 );
+};
+//CHECK: -CXXConstructorDecl {{.*}} x3 'void (const x3 &) __generic'
+template 
+x3::x3(const x3 ) {}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4526,6 +4526,14 @@
   return Result;
 }
 
+/// Helper to deduce addr space of a pointee type in OpenCL mode.
+/// If the type is updated it will be overwritten in PointeeType param.
+static void deduceOpenCLPointeeAddrSpace(Sema , QualType ) 
{
+  if (PointeeType.getAddressSpace() == LangAS::Default)
+PointeeType = SemaRef.Context.getAddrSpaceQualType(PointeeType,
+   LangAS::opencl_generic);
+}
+
 template
 QualType TreeTransform::TransformPointerType(TypeLocBuilder ,
   PointerTypeLoc TL) {
@@ -4534,6 +4542,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (PointeeType->getAs()) {
 // A dependent pointer type 'T *' has is being transformed such
@@ -4572,6 +4583,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != TL.getPointeeLoc().getType()) {
@@ -4601,6 +4615,9 @@
   if (PointeeType.isNull())
 return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != T->getPointeeTypeAsWritten()) {


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- test/SemaOpenCLCXX/address-space-deduction.cl
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -38,3 +38,43 @@
   int foo[10];
   xxx([0]);
 }
+
+// Deducing addr spaces for template specialization is fine
+// addr space of template arg can't affecting the addr space
+// of specialization
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= 'x1 &(const x1 &) __generic'
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (x1 *) __generic'
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1 *xx, __global x2 *bar) {
+  bar->foo(xx);
+}
+
+template 
+class x3 : public T {
+public:
+  //CHECK: -CXXConstructorDecl {{.*}} x3 'void (const x3 &) __generic'
+  x3(const x3 );
+};
+//CHECK: -CXXConstructorDecl {{.*}} x3 'void (const x3 &) __generic'
+template 
+x3::x3(const x3 ) {}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4526,6 +4526,14 @@
   return Result;
 }
 
+/// Helper to deduce addr space of a pointee type in OpenCL mode.
+/// If the type is updated it will be overwritten in PointeeType param.
+static void deduceOpenCLPointeeAddrSpace(Sema , QualType ) {
+  if (PointeeType.getAddressSpace() == LangAS::Default)

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaType.cpp:7421
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() && (!T->isPointerType() && !T->isReferenceType() &&

Anastasia wrote:
> rjmccall wrote:
> > I don't understand what you're saying here.  Why does inference depend on 
> > whether the type is a template specialization?  And what does this have to 
> > do with template arguments?  Also, address spaces in template arguments are 
> > definitely part of the template argument and affect which specialization 
> > you're naming.
> What I am trying to say here is that an address space of a template argument 
> isn't used as an address space of a template specialization and therefore we 
> can deduce the address space of a template specialization since it's not 
> going to be provided during the template instantiation.
> 
> Let's say we have specialization `MyClass`. The address space of `T` has 
> nothing to do with the address space of `MyClass`. They are different. 
> Therefore if the address space of `MyClass` is not provided explicitly it 
> is ok to deduce it.
> 
> Does it make sense?
Of course the address space of `T` has nothing to do with the address space of 
`MyClass`, but that's true of literally every type, and you don't need to 
add special cases checking for specific type spellings.

Why don't you just never infer address spaces on dependent types and then infer 
them as necessary during instantiation?  Why is it important to infer address 
spaces on any dependent type in the template pattern?


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 2 inline comments as done and 2 inline comments as done.
Anastasia added inline comments.



Comment at: include/clang/AST/Type.h:6512
+inline bool Type::isTemplateSpecializationType() const {
+  return isa(this);
+}

rjmccall wrote:
> This is a sugar type.  What are you trying to do?
I just need a helper function to identify this type in the addr space deduction 
logic below. Do you think we shouldn't expose it as interface? 

In fact I am now adding `isInjectedClassNameType` for the same reason to cover 
another similar test case.



Comment at: lib/Sema/SemaType.cpp:7421
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() && (!T->isPointerType() && !T->isReferenceType() &&

rjmccall wrote:
> I don't understand what you're saying here.  Why does inference depend on 
> whether the type is a template specialization?  And what does this have to do 
> with template arguments?  Also, address spaces in template arguments are 
> definitely part of the template argument and affect which specialization 
> you're naming.
What I am trying to say here is that an address space of a template argument 
isn't used as an address space of a template specialization and therefore we 
can deduce the address space of a template specialization since it's not going 
to be provided during the template instantiation.

Let's say we have specialization `MyClass`. The address space of `T` has 
nothing to do with the address space of `MyClass`. They are different. 
Therefore if the address space of `MyClass` is not provided explicitly it is 
ok to deduce it.

Does it make sense?



Comment at: lib/Sema/SemaType.cpp:7421
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() &&

If you think it's reasonable the same would apply to `InjectedClassNameType`.


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 208993.
Anastasia marked 2 inline comments as done.
Anastasia added a comment.

- Added handling of similar test case with `InjectedClassNameType`


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

https://reviews.llvm.org/D64400

Files:
  include/clang/AST/Type.h
  lib/Sema/SemaType.cpp
  test/SemaOpenCLCXX/address-space-deduction.cl


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- test/SemaOpenCLCXX/address-space-deduction.cl
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -38,3 +38,41 @@
   int foo[10];
   xxx([0]);
 }
+
+// Deducing addr spaces for template specialization is fine
+// addr space of template arg can't affecting the addr space
+// of specialization
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic 
x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1 *xx, __global x2 *bar) {
+  bar->foo(xx);
+}
+
+template 
+class x3 : public T {
+public:
+  //CHECK: -CXXConstructorDecl {{.*}} x3 'void (const __generic x3 &) 
__generic'
+  x3(const x3 );
+};
+//CHECK: -CXXConstructorDecl {{.*}} 'void (const __generic x3 &) __generic'
+template 
+x3::x3(const x3 ) {}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7414,9 +7414,14 @@
   (T->isVoidType() && !IsPointee) ||
   // Do not deduce addr spaces for dependent types because they might end
   // up instantiating to a type with an explicit address space qualifier.
-  // Expect for pointer or reference types because the addr space in
-  // template argument can only belong to a pointee.
-  (T->isDependentType() && !T->isPointerType() && !T->isReferenceType()) ||
+  // Except for:
+  // - pointer or reference types because the addr space in template
+  //   argument can only belong to a pointee.
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() &&
+   (!T->isPointerType() && !T->isReferenceType() &&
+!T->isTemplateSpecializationType() && !T->isInjectedClassNameType())) 
||
   // Do not deduce addr space of decltype because it will be taken from
   // its argument.
   T->isDecltypeType() ||
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1981,6 +1981,8 @@
   bool isObjCBoxableRecordType() const;
   bool isInterfaceType() const;
   bool isStructureOrClassType() const;
+  bool isTemplateSpecializationType() const;
+  bool isInjectedClassNameType() const;
   bool isUnionType() const;
   bool isComplexIntegerType() const;// GCC _Complex integer type.
   bool isVectorType() const;// GCC vector type.
@@ -6507,6 +6509,14 @@
   return isa(this);
 }
 
+inline bool Type::isTemplateSpecializationType() const {
+  return isa(this);
+}
+
+inline bool Type::isInjectedClassNameType() const {
+  return isa(this);
+}
+
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   inline bool Type::is##Id##Type() const { \
 return isSpecificBuiltinType(BuiltinType::Id); \


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- test/SemaOpenCLCXX/address-space-deduction.cl
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -38,3 +38,41 @@
   int foo[10];
   xxx([0]);
 }
+
+// Deducing addr spaces for template specialization is fine
+// addr space of template arg can't affecting the addr space
+// of specialization
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1 *xx, __global x2 *bar) {
+  bar->foo(xx);
+}
+
+template 
+class x3 : public T {
+public:
+  //CHECK: -CXXConstructorDecl {{.*}} x3 'void (const __generic x3 &) __generic'
+  x3(const x3 );
+};
+//CHECK: -CXXConstructorDecl {{.*}} 'void (const __generic x3 &) __generic'
+template 
+x3::x3(const x3 ) {}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7414,9 +7414,14 @@
   (T->isVoidType() && !IsPointee) ||
   // 

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:6512
+inline bool Type::isTemplateSpecializationType() const {
+  return isa(this);
+}

This is a sugar type.  What are you trying to do?



Comment at: lib/Sema/SemaType.cpp:7417
   // up instantiating to a type with an explicit address space qualifier.
-  // Expect for pointer or reference types because the addr space in
-  // template argument can only belong to a pointee.
-  (T->isDependentType() && !T->isPointerType() && !T->isReferenceType()) ||
+  // Expect for:
+  // - pointer or reference types because the addr space in template

"Except"?



Comment at: lib/Sema/SemaType.cpp:7421
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() && (!T->isPointerType() && !T->isReferenceType() &&

I don't understand what you're saying here.  Why does inference depend on 
whether the type is a template specialization?  And what does this have to do 
with template arguments?  Also, address spaces in template arguments are 
definitely part of the template argument and affect which specialization you're 
naming.


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

https://reviews.llvm.org/D64400



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

[PATCH] D64400: [OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: rjmccall, mantognini.
Herald added subscribers: ebevhan, yaxunl.

Addr space of a template arg doesn't affect the `QualType` of template 
specialization. Therefore addr space of  a template specialization can be 
deduced during parsing of template definition directly.


https://reviews.llvm.org/D64400

Files:
  include/clang/AST/Type.h
  lib/Sema/SemaType.cpp
  test/SemaOpenCLCXX/address-space-deduction.cl


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- test/SemaOpenCLCXX/address-space-deduction.cl
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -38,3 +38,33 @@
   int foo[10];
   xxx([0]);
 }
+
+// Deducing addr spaces for template specialization is fine
+// addr space of template arg can't affecting the addr space
+// of specialization
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic 
x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1* xx, __global x2* bar)
+{
+  bar->foo(xx);
+}
+
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7414,9 +7414,13 @@
   (T->isVoidType() && !IsPointee) ||
   // Do not deduce addr spaces for dependent types because they might end
   // up instantiating to a type with an explicit address space qualifier.
-  // Expect for pointer or reference types because the addr space in
-  // template argument can only belong to a pointee.
-  (T->isDependentType() && !T->isPointerType() && !T->isReferenceType()) ||
+  // Expect for:
+  // - pointer or reference types because the addr space in template
+  //   argument can only belong to a pointee.
+  // - template specialization as addr space in template argument doesn't
+  //   affect specialization.
+  (T->isDependentType() && (!T->isPointerType() && !T->isReferenceType() &&
+!T->isTemplateSpecializationType())) ||
   // Do not deduce addr space of decltype because it will be taken from
   // its argument.
   T->isDecltypeType() ||
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1981,6 +1981,7 @@
   bool isObjCBoxableRecordType() const;
   bool isInterfaceType() const;
   bool isStructureOrClassType() const;
+  bool isTemplateSpecializationType() const;
   bool isUnionType() const;
   bool isComplexIntegerType() const;// GCC _Complex integer type.
   bool isVectorType() const;// GCC vector type.
@@ -6507,6 +6508,10 @@
   return isa(this);
 }
 
+inline bool Type::isTemplateSpecializationType() const {
+  return isa(this);
+}
+
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   inline bool Type::is##Id##Type() const { \
 return isSpecificBuiltinType(BuiltinType::Id); \


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- test/SemaOpenCLCXX/address-space-deduction.cl
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -38,3 +38,33 @@
   int foo[10];
   xxx([0]);
 }
+
+// Deducing addr spaces for template specialization is fine
+// addr space of template arg can't affecting the addr space
+// of specialization
+
+template 
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1 &(const __generic x1 &) __generic'
+  x1& operator=(const x1& xx) {
+y = xx.y;
+return *this;
+  }
+  int y;
+};
+
+template 
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1 *) __generic'
+  void foo(x1* xx) {
+m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1 [2]'
+  x1 m[2];
+};
+
+void bar(__global x1* xx, __global x2* bar)
+{
+  bar->foo(xx);
+}
+
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7414,9 +7414,13 @@
   (T->isVoidType() && !IsPointee) ||
   // Do not deduce addr spaces for dependent types because they might end
   // up instantiating to a type with an explicit address space qualifier.
-  // Expect for pointer or reference types because the addr space in
-  // template argument can only belong to a pointee.
-  (T->isDependentType() && !T->isPointerType() && !T->isReferenceType()) ||
+  // Expect for:
+  // - pointer or reference types because the addr space in template
+  //   argument can only belong to a pointee.
+  // - template specialization as addr space in template