[Patch][OpenCL] CL2.0 atomic types, diagnostics

[Sahasrabuddhe, Sameer]

 > 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 at -16 {{use of undeclared identifier 'atomic_size_t'}}
 >  // expected-error at -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.