Page MenuHomePhabricator

[OpenCL] Diagnose conflicting address spaces between template definition and its instantiation
ClosedPublic

Authored by Anastasia on Nov 30 2018, 8:16 AM.

Details

Summary

Added new diagnostics when templates are instantiated with different address space from the one provided in definition.

This also prevents deducing generic address space in pointer type of templates to allow giving them concrete address space.

Diff Detail

Repository
rC Clang

Event Timeline

Anastasia created this revision.Nov 30 2018, 8:16 AM
rjmccall added inline comments.Nov 30 2018, 10:05 AM
lib/Sema/SemaType.cpp
7235

I don't understand what you're trying to do here. Template arguments may need to be directly qualified with an address-space sometimes, and if you prevent that unconditionally you're going to break all sorts of things, like partially-specializing a template based on the presence of an address-space qualifier.

Anastasia marked an inline comment as done.Dec 3 2018, 2:59 AM
Anastasia added inline comments.
lib/Sema/SemaType.cpp
7235

I want to prevent deduction of address spaces for template arguments (when they are not specified explicitly).

Without this change, this example won't compile:

template <typename T>
void foo() {
  static __global T i;
}
foo<int>(); // error because int here is deduced to __private int (so i will have conflicting addr space quals)

But I think it's perfectly reasonable to compile this example because the addr space qual of i is specified to be __global.

Basically I am just trying to fix OpenCL C deduction rules that didn't account for the logic of templates.

Okay. What you're saying is that the current OpenCL C++ compiler is implicitly adding an address-space qualifier at the top level to explicit template arguments. I agree that it should not be doing that.

Can you explain why the changes to TreeTransform are required?

Can you explain why the changes to TreeTransform are required?

When address spaces of template parameter and argument are given explicitly they might mismatch. For example, foo1 (in the test) is instantiated with __local int but the template parameter of foo1 is __global T. Currently clang fails to compile this with ICE while rebuilding the type that has multiple inconsistent addr space qualifiers. My patch fixes it, by giving an error instead of attempting to rebuild the type, that the addr space qualifiers mismatch.

There is another example of the same issue - foo3, but with non-pointer type.

rjmccall accepted this revision.Dec 4 2018, 10:48 AM

LGTM with minor revisions.

lib/Sema/SemaType.cpp
7206

I think a better comment here would be something like "Don't deduce address spaces for dependent types because they might end up instantiating to a type with a different address space qualifier."

7235

Okay. Please use braces instead of a ;.

This revision is now accepted and ready to land.Dec 4 2018, 10:48 AM
Anastasia updated this revision to Diff 176793.Dec 5 2018, 4:19 AM

Added last corrections before committing.

This revision was automatically updated to reflect the committed changes.