[PATCH] D32977: [OpenCL] Emit function-scope variable in constant address space as static variable

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:10286
+          // these variables must be a compile time constant.
+          VDecl->getType().getAddressSpace() == LangAS::opencl_constant)
         CheckForConstantInitializer(Init, DclT);
----------------
rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Should this rule apply even in C++ mode?  I can't remember if there are any OpenCL/C++ hybrids.
> > No. This rule (static local variable needs to be initialised with compile-time constant) only applies to C and OpenCL. 
> > In C++ static local variable can be initialised with non-compile-time constant, in which case Clang will emit atomic instructions to make sure it is only initialised once.
> > 
> > Currently OpenCL 2.2 defines OpenCL C++ but clang does not support it.
> Yes, I understand that C++ generally allows static locals to be lazily initialized, and that that rule would (probably) still apply to ordinary static locals in OpenCL C++.  However, I would expect that OpenCL C++ rule is that __constant local variables still need to be statically initialized, because there's no plausible way in the OpenCL implementation model to actually put lazily-initialized variables in the constant address space.  Assuming that's correct, then I would recommend reworking this whole chain of conditions to:
> 
>   // Don't check the initializer if the declaration is malformed.
>   if (VDecl->isInvalidDecl()) {
>     // do nothing
> 
>   // OpenCL __constant locals must be constant-initialized, even in OpenCL C++.
>   } else if (VDecl->getType().getAddressSpace() == LangAS::opencl_constant) {
>     CheckForConstantInitializer(Init, DclT);
> 
>   // Otherwise, C++ does not restrict the initializer.
>   } else if (getLangOpts().CPlusPlus) {
>     // do nothing
> 
>   // C99 6.7.8p4: All the expressions in an initializer for an object that has
>   // static storage duration shall be constant expressions or string literals.
>   } else if (VDecl->getStorageClass() == SC_Static) {
>     CheckForConstantInitializer(Init, DclT);
> 
>   // C89 is stricter than C99 for aggregate initializers.
>   // C89 6.5.7p3: All the expressions [...] in an initializer list
>   // for an object that has aggregate or union type shall be
>   // constant expressions.
>   } else if (!getLangOpts().C99 && VDecl->getType()->isAggregateType() && isa<InitListExpr>(Init)) {
>     Expr *Culprit;
>     if (!Init->isConstantInitializer(Context, false, &Culprit)) {
>       ...
>     }
>   }
Agree that even OpenCL C++ is unable to lazy initialise function-scope var in constant addr space. Will do.


https://reviews.llvm.org/D32977