Page MenuHomePhabricator

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

Authored by Anastasia on Jul 9 2019, 4:38 AM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

Anastasia created this revision.Jul 9 2019, 4:38 AM
rjmccall added inline comments.Jul 9 2019, 11:09 PM
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 updated this revision to Diff 208993.Jul 10 2019, 9:43 AM
Anastasia marked 2 inline comments as done.
  • Added handling of similar test case with InjectedClassNameType
Anastasia marked 2 inline comments as done and 2 inline comments as done.Jul 10 2019, 9:46 AM
Anastasia added inline comments.
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 added inline comments.Jul 10 2019, 10:21 AM
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 updated this revision to Diff 209205.Jul 11 2019, 6:43 AM
  • Moved addr space inference of pointee type into template instantiation.
Anastasia marked an inline comment as done.Jul 11 2019, 6:53 AM
Anastasia added inline comments.
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.

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.

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.

This revision is now accepted and ready to land.Jul 12 2019, 12:17 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 6:03 AM