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