[PATCH] D82781: [OpenCL] Fix missing address space deduction in template variables

[Ole Strohm via Phabricator via cfe-commits]

olestrohm created this revision.
olestrohm added reviewers: Anastasia, rjmccall.
olestrohm added a project: clang.
Herald added a subscriber: yaxunl.

This patch fixes two instances of not deducing address spaces for global template variables.
I've added OpenCL address space deduction to the functions responsible for specializing template variable declarations,
and while this does give the correct address spaces, it is not optimal, as this same address space is deduced at least three times for a single variable.
It would be better if the correct type was correctly passed forward through the phases, which I've explained in the comments.
Since the variable is templated I'm not entirely certain if this is feasible or worthwhile compared to the simple solution of rededucing the address space several times.

I've also included this an example of this in the test for address space deductions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82781

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaOpenCLCXX/address-space-deduction.cl


Index: clang/test/SemaOpenCLCXX/address-space-deduction.cl
===================================================================
--- clang/test/SemaOpenCLCXX/address-space-deduction.cl
+++ clang/test/SemaOpenCLCXX/address-space-deduction.cl
@@ -5,6 +5,10 @@
 //CHECK: |-VarDecl {{.*}} foo 'const __global int'
 constexpr int foo = 0;
 
+//CHECK: `-VarTemplateSpecializationDecl {{.*}} used foo1 '__global long':'__global long' cinit
+template <typename T>
+T foo1 = 0;
+
 class c {
 public:
   //CHECK: `-VarDecl {{.*}} foo2 'const __global int'
@@ -111,4 +115,5 @@
   t3(&x);
   t4(&p);
   t5(&p);
+  long f1 = foo1<long>;
 }
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3625,6 +3625,13 @@
   if (InsertPos)
     VarTemplate->AddSpecialization(Var, InsertPos);
 
+  // FIXME: This may not be the best approach, as the correct type (including
+  // address space) is available in D, but the type in D may not be reliable
+  // in every situation.
+  // This approach was copied from TemplateDeclInstantiator::VisitVarDecl
+  if (SemaRef.getLangOpts().OpenCL)
+     SemaRef.deduceOpenCLAddressSpace(Var);
+
   // Substitute the nested name specifier, if any.
   if (SubstQualifier(D, Var))
     return nullptr;
@@ -4803,6 +4810,13 @@
   // Instantiate the initializer.
   InstantiateVariableInitializer(VarSpec, PatternDecl, TemplateArgs);
 
+  // FIXME: This may not be the best approach, as the correct type (including
+  // address space) is available in PatternDecl, but this type may not be
+  // reliable in every situation.
+  // This approach was copied from TemplateDeclInstantiator::VisitVarDecl
+  if (getLangOpts().OpenCL)
+     deduceOpenCLAddressSpace(VarSpec);
+
   return VarSpec;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82781.274144.patch
Type: text/x-patch
Size: 1882 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200629/9e75de82/attachment-0001.bin>