[OpenCL][PR42390] Deduce addr space for templ specialization

[Anastasia]

Summary

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.

[rjmccall]

include/clang/AST/Type.h

6512 ↗

(On Diff #208637)

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

lib/Sema/SemaType.cpp

7417 ↗

(On Diff #208637)

"Except"?

7421 ↗

(On Diff #208637)

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.

[Anastasia]

• Added handling of similar test case with InjectedClassNameType

[Anastasia]

include/clang/AST/Type.h

6512 ↗

(On Diff #208637)

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.

lib/Sema/SemaType.cpp

7421 ↗

(On Diff #208993)

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

7421 ↗

(On Diff #208637)

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<T>. The address space of T has nothing to do with the address space of MyClass<T>. They are different. Therefore if the address space of MyClass<T> is not provided explicitly it is ok to deduce it.

Does it make sense?

[rjmccall]

lib/Sema/SemaType.cpp

7421 ↗

(On Diff #208637)

Of course the address space of T has nothing to do with the address space of MyClass<T>, 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?

[Anastasia]

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

[Anastasia]

lib/Sema/SemaType.cpp

7421 ↗

(On Diff #208637)

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.

[rjmccall]

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.

[Anastasia]

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.

[rjmccall]

Okay.

cfe/trunk/lib/Sema/TreeTransform.h

QualType TreeTransform<Derived>::TransformDecayedType(TypeLocBuilder &TLB,
   if (getDerived().AlwaysRebuild() ||
       OriginalType != TL.getOriginalLoc().getType())
     Result = SemaRef.Context.getDecayedType(OriginalType);
   TLB.push<DecayedTypeLoc>(Result);
   // Nothing to set for DecayedTypeLoc.
   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 &SemaRef, QualType &PointeeType) {
+  if (PointeeType.getAddressSpace() == LangAS::Default)
+    PointeeType = SemaRef.Context.getAddrSpaceQualType(PointeeType,
+                                                       LangAS::opencl_generic);
+}
+
 template<typename Derived>
 QualType TreeTransform<Derived>::TransformPointerType(TypeLocBuilder &TLB,
                                                       PointerTypeLoc TL) {
   QualType PointeeType
     = getDerived().TransformType(TLB, TL.getPointeeLoc());
   if (PointeeType.isNull())
     return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+    deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (PointeeType->getAs<ObjCObjectType>()) {
     // A dependent pointer type 'T *' has is being transformed such
     // that an Objective-C class type is being replaced for 'T'. The
     // resulting pointer type is an ObjCObjectPointerType, not a
     // PointerType.
     Result = SemaRef.Context.getObjCObjectPointerType(PointeeType);
 

 QualType
 TreeTransform<Derived>::TransformBlockPointerType(TypeLocBuilder &TLB,
                                                   BlockPointerTypeLoc TL) {
   QualType PointeeType
     = getDerived().TransformType(TLB, TL.getPointeeLoc());
   if (PointeeType.isNull())
     return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+    deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
       PointeeType != TL.getPointeeLoc().getType()) {
     Result = getDerived().RebuildBlockPointerType(PointeeType,
                                                   TL.getSigilLoc());
     if (Result.isNull())
       return QualType();
   }
TreeTransform<Derived>::TransformReferenceType(TypeLocBuilder &TLB,
                                                ReferenceTypeLoc TL) {
   const ReferenceType *T = TL.getTypePtr();
 
   // Note that this works with the pointee-as-written.
   QualType PointeeType = getDerived().TransformType(TLB, TL.getPointeeLoc());
   if (PointeeType.isNull())
     return QualType();
 
+  if (SemaRef.getLangOpts().OpenCL)
+    deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
+
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
       PointeeType != T->getPointeeTypeAsWritten()) {
     Result = getDerived().RebuildReferenceType(PointeeType,
                                                T->isSpelledAsLValue(),
                                                TL.getSigilLoc());
     if (Result.isNull())
       return QualType();

cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl

 //CHECK: TypeAliasDecl {{.*}} alias_c1_ptr '__generic c1 *'
 using alias_c1_ptr = c1 *;
 
 struct c2 {
   alias_c1 y;
   alias_c1_ptr ptr = &y;
 };
 
+
+// Addr spaces for pointee of dependent types are not deduced
+// during parsing but during template instantiation instead.
+
+template <class T>
+struct x1 {
+//CHECK: -CXXMethodDecl {{.*}} operator= 'x1<T> &(const x1<T> &) __generic'
+//CHECK: -CXXMethodDecl {{.*}} operator= '__generic x1<int> &(const __generic x1<int> &) __generic'
+  x1<T>& operator=(const x1<T>& xx) {
+    y = xx.y;
+    return *this;
+  }
+  int y;
+};
+
+template <class T>
+struct x2 {
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (x1<T> *) __generic'
+//CHECK: -CXXMethodDecl {{.*}} foo 'void (__generic x1<int> *) __generic'
+  void foo(x1<T>* xx) {
+    m[0] = *xx;
+  }
+//CHECK: -FieldDecl {{.*}}  m 'x1<int> [2]'
+  x1<T> m[2];
+};
+
+void bar(__global x1<int> *xx, __global x2<int> *bar) {
+  bar->foo(xx);
+}
+
+template <typename T>
+class x3 : public T {
+public:
+  //CHECK: -CXXConstructorDecl {{.*}} x3<T> 'void (const x3<T> &) __generic'
+  x3(const x3 &t);
+};
+//CHECK: -CXXConstructorDecl {{.*}} x3<T> 'void (const x3<T> &) __generic'
+template <typename T>
+x3<T>::x3(const x3<T> &t) {}