@Sameer or anyone else, are there any more comments to this patch? Thanks, Anastasia
-----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Anastasia Stulova Sent: 12 February 2015 17:16 To: 'Sahasrabuddhe, Sameer'; [email protected] Subject: RE: [Patch][OpenCL] CL2.0 atomic types, diagnostics Hi Sameer, Patch updated with your comments! A few issues here: 1. I am not entirely sure what extra steps we need to do in the compiler for initialization of atomic variables, since program scope atomic variables can only be initialized with constant values and atomic_init function seems to have no restrictions on the type of the value passed in. 2. 'atomic_int' (aka '_Atomic(int)') comes from standard TypePrinter, because we implement CL2.0 atomics by aliasing them with C11 atomics. That saves us a lot of implementation effort and avoids code duplication. Regards, Anastasia -----Original Message----- From: Sahasrabuddhe, Sameer [mailto:[email protected]] Sent: 16 December 2014 05:24 To: Anastasia Stulova; [email protected] Subject: Re: [Patch][OpenCL] CL2.0 atomic types, diagnostics > diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td > index 75e78bf..9f077c9 100644 > --- a/include/clang/Basic/DiagnosticSemaKinds.td > +++ b/include/clang/Basic/DiagnosticSemaKinds.td > @@ -7175,6 +7175,9 @@ def err_opencl_return_value_with_address_space : Error< > "return value cannot be qualified with address space">; > def err_opencl_constant_no_init : Error< > "variable in constant address space must be initialized">; > +def err_atomic_init_constant : Error < > + "atomic variable can only be > assigned to a compile time constant" > + " in the declaration statement in the program scope">; Could we rephrase this? Perhaps "assigning directly to atomic objects is not allowed". The assumption is that initialization will be checked elsewhere, so it does not have to be mentioned here. Should that be part of this patch? > } // end of sema category > > let CategoryName = "OpenMP Issue" in { > diff --git > a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp > index 76e3612..b403e53 > 100644 > --- a/lib/Sema/SemaExpr.cpp > +++ b/lib/Sema/SemaExpr.cpp > @@ > -9489,6 +9489,20 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, > return ExprError(); > } > > + if (getLangOpts().OpenCL) { > + // OpenCLC v2.0 s6.13.11.1 allows atomic variables to be initialized by > + // the ATOMIC_VAR_INIT macro. > + if (LHSExpr->getType()->isAtomicType() || > + RHSExpr->getType()->isAtomicType()) { > + SourceRange SR(LHSExpr->getLocStart(), RHSExpr->getLocEnd()); > + if (BO_Assign == Opc) > + Diag(OpLoc, diag::err_atomic_init_constant) << SR; I think the recommended style is "Opc == BO_Assign" (but I am not sure). > + else > + ResultTy = InvalidOperands(OpLoc, LHS, RHS); > + return ExprError(); > + } > + } > + > switch (Opc) { > case BO_Assign: > ResultTy = CheckAssignmentOperands(LHS.get(), RHS, OpLoc, QualType()); > @@ -9959,6 +9973,14 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc, > ExprValueKind VK = VK_RValue; > ExprObjectKind OK = OK_Ordinary; > QualType resultType; > + if (getLangOpts().OpenCL) { > + // The only legal unary operation for atomics is '&'. > + if (Opc != UO_AddrOf && InputExpr->getType()->isAtomicType()) { > + return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr) > + << InputExpr->getType() > + << Input.get()->getSourceRange()); > + } > + } > switch (Opc) { > case UO_PreInc: > case UO_PreDec: > diff --git a/test/Parser/opencl-atomics-cl20.cl b/test/Parser/opencl-atomics-cl20.cl > index d5cecd1..4ba699b1 100644 > --- a/test/Parser/opencl-atomics-cl20.cl > +++ b/test/Parser/opencl-atomics-cl20.cl > @@ -31,3 +31,16 @@ void atomic_types_test() { > // expected-error@-16 > {{use of undeclared identifier 'atomic_size_t'}} > // expected-error@-16 > {{use of undeclared identifier 'atomic_ptrdiff_t'}} > #endif > + > > +#ifdef CL20 > +void foo(atomic_int * ptr) {} > +void atomic_ops_test() { > > + atomic_int i; > + foo(&i); > +// OpenCL v2.0 s6.13.11.8, arithemtic > operation are not permitted on atomic types. > + i++; // expected-error {{invalid argument type 'atomic_int' (aka '_Atomic(int)') to unary expression}} > + i = 1; // expected-error {{atomic variable can only be assigned to a > compile time constant in the declaration statement in the program scope}} > > + i += 1; // expected-error {{invalid operands to binary expression > ('atomic_int' (aka '_Atomic(int)') and 'int')}} > + i = i + i; // > expected-error {{invalid operands to binary expression ('atomic_int' (aka > '_Atomic(int)') and 'atomic_int')}} Nitpick: The "aka '_Atomic(int)'" isn't really true for OpenCL 2.0! But I don't have a strong opinion either way. > +} > +#endif > Sameer. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
