Anastasia accepted this revision.
Anastasia added a comment.
LGTM!
================
Comment at: lib/Sema/SemaDecl.cpp:10286
+ // these variables must be a compile time constant.
+ VDecl->getType().getAddressSpace() == LangAS::opencl_constant)
CheckForConstantInitializer(Init, DclT);
----------------
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.
https://reviews.llvm.org/D32977
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
