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);
----------------
Anastasia wrote:
> yaxunl wrote:
> > 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.
> I think the original way would be much simpler to read and understand though.
> 
> To be honest I wouldn't complicate things now for the feature we don't 
> support. I feel OpenCL C++ should be represented as a separate LangOpt since 
> there are some places that will require special handling due to deviation 
> from C++. I would rather extend things later in more systematic way.
I will delete the comment about OpenCL C++ when committing.


https://reviews.llvm.org/D32977



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to