[OpenCL]Allowing explicit conversion of "0" to event_t type

[ashi1]

Summary

This patch will allow the cast of 0 to event_t type.

[pxli168]

foo((event_t)0);

Is above use have been hint by some test cases?

include/clang/Basic/DiagnosticSemaKinds.td

7679 ↗

(On Diff #48966)

Please do not use DOS format.

lib/Sema/SemaCast.cpp

2316 ↗

(On Diff #48966)

1. Comments start with capital.
2. End a sentence with a period.
3. Give spec reference here, start with:

OpenCL v2.0 s6.13.10 - ....

2317 ↗

(On Diff #48966)

(Self.getLangOpts().OpenCL) -> Self.getLangOpts().OpenCL

2318 ↗

(On Diff #48966)

Indent.

2319 ↗

(On Diff #48966)

Sema has a context.
Self.getASTContext() ->Self.Context

2321 ↗

(On Diff #48966)

There is a CK_ZeroToOCLEvent, why not use that?

2324–2328 ↗

(On Diff #48966)

Indent.
You can try to use clang-format to help keep these right.

[pxli168]

lib/Sema/SemaCast.cpp

2324–2326 ↗

(On Diff #48966)

Need a test case for this.

[Anastasia]

Could you please change my role to the Subscriber and add Joey as a reviewer please.

[ashi1]

Joey Gouly or Joey Korkames?

Thanks,
Aaron

[Anastasia]

Joey Gouly!

Thanks!

[ashi1]

Revised with Xiuli Pan's comments.

[majnemer]

lib/Sema/SemaCast.cpp

2318 ↗

(On Diff #49219)

Please name this variable properly: http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

2322–2323 ↗

(On Diff #49219)

Don't have an else after return. http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

[• tstellarAMD]

lib/Sema/SemaCast.cpp

2317 ↗

(On Diff #49219)

A similar patch has been submitted and rejected several times:

https://www.mail-archive.com/[e-mail]/msg91067.html

http://marc.info/?l=cfe-commits&m=141198505414824&w=2

These were all before OpenCL 2.0. The 1.2 spec does not allow this, so I think this should be guarded by an OpenCL version check.

[yaxunl]

lib/Sema/SemaCast.cpp

2317 ↗

(On Diff #49219)

I opened a bug at khronos bugzilla requesting clarification of the issue:

https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15609

I will keep you updated.

[yaxunl]

lib/Sema/SemaCast.cpp

2317 ↗

(On Diff #49219)

OpenCL WG has clarified that explicit or implicit cast of 0 to event_t is allowed. It does not make sense to only allow implicit cast but not allow explicit cast. So I think we should move on with this patch.

[ashi1]

Added changes for comments from majnemer.

[ashi1]

Modified the testcase. Tested this diff on clang-test and it passes.

[joey]

LGTM!

[yaxunl]

LGTM. Thanks!

[ashi1]

Xiuli, are you OK with this patch?

[pxli168]

Sorry about late reply.
LGTM!

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def err_event_t_addr_space_qual : Error<
   "the event_t type can only be used with __private address space qualifier">;
 def err_expected_kernel_void_return_type : Error<
   "kernel must have void return type">;
 def err_sampler_argument_required : Error<
   "sampler_t variable required - got %0">;
 def err_wrong_sampler_addressspace: Error<
   "sampler type cannot be used with the __local and __global address space qualifiers">;
+def error_opencl_cast_non_zero_to_event_t : Error<
+  "cannot cast non-zero value '%0' to 'event_t'">;
 def err_opencl_global_invalid_addr_space : Error<
   "%select{program scope|static local|extern}0 variable must reside in %1 address space">;
 def err_missing_actual_pipe_type : Error<
   "missing actual type specifier for pipe">;
 def err_reference_pipe_type : Error <
   "pipes packet types cannot be of reference type">;
 def err_opencl_no_main : Error<"%select{function|kernel}0 cannot be called 'main'">;
 def err_opencl_kernel_attr :

cfe/trunk/lib/Sema/SemaCast.cpp

if (DestRecordTy && DestRecordTy->getDecl()->isUnion()) {
           << SrcType << SrcExpr.get()->getSourceRange();
         SrcExpr = ExprError();
         return;
       }
       Kind = CK_ToUnion;
       return;
     }
 
+    // OpenCL v2.0 s6.13.10 - Allow casts from '0' to event_t type.
+    if (Self.getLangOpts().OpenCL && DestType->isEventT()) {
+      llvm::APSInt CastInt;
+      if (SrcExpr.get()->EvaluateAsInt(CastInt, Self.Context)) {
+        if (0 == CastInt) {
+          Kind = CK_ZeroToOCLEvent;
+          return;
+        }
+        Self.Diag(OpRange.getBegin(),
+                  diag::error_opencl_cast_non_zero_to_event_t)
+                  << CastInt.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.

cfe/trunk/test/CodeGenOpenCL/event_t.cl

 // RUN: %clang_cc1 %s -emit-llvm -o - -O0 | FileCheck %s
 
 void foo(event_t evt);
 
 void kernel ker() {
   event_t e;
 // CHECK: alloca %opencl.event_t*,
   foo(e);
 // CHECK: call {{.*}}void @foo(%opencl.event_t* %
   foo(0);
 // CHECK: call {{.*}}void @foo(%opencl.event_t* null)
+  foo((event_t)0);
+// CHECK: call {{.*}}void @foo(%opencl.event_t* null)
 }

cfe/trunk/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 kernel ker(event_t argevt) { // expected-error {{'event_t' cannot be used as the type of a kernel parameter}}
   event_t e;
   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(5); // expected-error {{passing 'int' to parameter of incompatible type 'event_t'}}
+  foo((event_t)1); // expected-error {{cannot cast non-zero value '1' to 'event_t'}}
 }