Permit OpenCL event_t's to be constructed from integer value 0.

[sheredom]

Summary

The OpenCL specification and conformance tests have code similar to;

void func(event_t e);
...
func((event_t)0);

Clang (in OpenCL language mode) currently supports the above, but without the explicit cast, like;

func(0);

But doesn't allow the explicit cast. This patch fixes this, and also adds in a more descriptive error for when a user does;

(event_t)5;

and also adds in some more expected results to the OpenCL event_t tests.

Any comments or suggestions would be greatly appreciated!

[sheredom]

Also, ran check-all tests, results as below;

Neils-MacBook-Air:build neil$ ninja check-all
[1/1] Running all regression tests
lit.py: lit.cfg:196: note: using clang: '/Users/neil/repo/llvm/build/./bin/clang'
lit.py: lit.cfg:440: note: using SDKROOT: '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk'
-- Testing: 16740 tests, 4 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 403.83s
  Expected Passes    : 16501
  Expected Failures  : 74
  Unsupported Tests  : 165

[rsmith]

Can you show me where in the OpenCL spec this language rule is described? I can't find any allowance for casting zero to event_t.

include/clang/Basic/DiagnosticSemaKinds.td

6788

err_, not error_.

6789

Maybe '%1' instead of 'event_t' here, so we get a better diagnostic if the target type is a typedef ("cannot cast non-zero value '1' to 'foo' (aka 'event_t')")?

lib/Sema/SemaCast.cpp

2241

Capital F in For.

2242

Remove redundant parens around OpenCL check.

2243

Should be IntValue

2244

EvaluateAsInt is inappropriate here. I'm not sure exactly what the language rule is, but this surely isn't it. Maybe take a look at TryOCLZeroEventInitialization in SemaInit.cpp.

2244–2245

Space between if and (.

2245

We don't do the constant-on-the-left thing in the Clang codebase. Also, operator! is more efficient than != 0 for APSInt.

2251

Both arms of this if/else return, factor that out?

[sheredom]

Thanks for the comments!

Its very obscure in the spec, but if you look at the functions that use event_t types, like async_work_group_copy it says 'The event argument can also be used to associate the async_work_group_copy with a previous async copy allowing an event to be shared by multiple async copies; otherwise event should be zero'.

[rsmith]

We already allow passing 0 in such cases. I don't see any provision for allowing explicit casts from 0 to event_t. The "Explicit Casts" section says nothing about it, for instance.

[sheredom]

I've seen behaviour similar to this being accepted by implementations, but you are correct in that the specification doesn't explicitly say that it is acceptable behaviour!

I've posted in the OpenCL forums here Clarification-on-event_t-usage-in-OpenCL-specification to see if I can get some clarification.

Thanks again for your help!

[sheredom]

Rejected my own review as specification is unclear on expected behaviour, will seek clarification on spec, then re-evaluate any intended patch.

include/clang/Basic/DiagnosticSemaKinds.td

def err_opencl_global_invalid_addr_space : Error<
   "global variables must have a constant address space qualifier">;
 def err_opencl_no_main : Error<"%select{function|kernel}0 cannot be called 'main'">;
 def err_opencl_kernel_attr :
   Error<"attribute %0 can only be applied to a kernel function">;
 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 error_opencl_cast_non_zero_to_event_t : Error<
+  "cannot cast non-zero value '%0' to 'event_t'">;
 } // end of sema category
 
 let CategoryName = "OpenMP Issue" in {
 // OpenMP support.
 def err_omp_expected_var_arg : Error<
   "%0 is not a global variable, static local variable or static data member">;
 def err_omp_expected_var_arg_suggest : Error<
   "%0 is not a global variable, static local variable or static data member; "

lib/Sema/SemaCast.cpp

if (DestRecordTy && DestRecordTy->getDecl()->isUnion()) {
           << SrcType << SrcExpr.get()->getSourceRange();
         SrcExpr = ExprError();
         return;
       }
       Kind = CK_ToUnion;
       return;
     }
 
+    // for OpenCL, allow casts from '0' to event_t type
+    if ((Self.getLangOpts().OpenCL) && DestType->isEventT()) {
+      llvm::APSInt intValue;
+      if(SrcExpr.get()->EvaluateAsInt(intValue, Self.getASTContext())) {
+        if(0 == intValue) {
+          return;
+        } else {
+          Self.Diag(OpRange.getBegin(), diag::error_opencl_cast_non_zero_to_event_t)
+            << intValue.toString(10) << SrcExpr.get()->getSourceRange();
+          SrcExpr = ExprError();
+          return;
+        }
+      }
+    }
+
     // Reject any other conversions to non-scalar types.
     Self.Diag(OpRange.getBegin(), diag::err_typecheck_cond_expect_scalar)
       << DestType << SrcExpr.get()->getSourceRange();
     SrcExpr = ExprError();
     return;
   }
 
   // The type we're casting to is known to be a scalar or vector.

test/SemaOpenCL/event_t.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 
 event_t glb_evt; // expected-error {{the event_t type cannot be used to declare a program scope variable}}
 
 constant struct evt_s {
   event_t evt;  // expected-error {{the event_t type cannot be used to declare a structure or union field}}
 } evt_str = {0};
 
-void foo(event_t evt); // expected-note {{passing argument to parameter 'evt' here}}
+void foo(event_t evt); // expected-note 2 {{passing argument to parameter 'evt' here}}
 
 void kernel ker(event_t argevt) { // expected-error {{'event_t' cannot be used as the type of a kernel parameter}}
   event_t e;
+  int i;
   constant event_t const_evt; // expected-error {{the event_t type can only be used with __private address space qualifier}}
   foo(e);
   foo(0);
+  foo((event_t)0);
+  foo(i); // expected-error {{passing 'int' to parameter of incompatible type 'event_t'}}
+  foo((event_t)i); // expected-error {{used type 'event_t' where arithmetic or pointer type is required}}
   foo(5); // expected-error {{passing 'int' to parameter of incompatible type 'event_t'}}
+  foo((event_t)5); // expected-error {{cannot cast non-zero value '5' to 'event_t'}}
 }