rjmccall added inline comments.
================
Comment at: lib/CodeGen/CGDecl.cpp:158
+ // variable in constant address space in OpenCL.
+ if (D.getStorageDuration() != SD_Automatic && !D.hasExternalStorage()) {
llvm::GlobalValue::LinkageTypes Linkage =
----------------
Please rearrange this function so that the D.hasExternalStorage() case below
comes first; that will then simplify this condition.
================
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:
> > 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)) {
...
}
}
https://reviews.llvm.org/D32977
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
