[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

Please do not use DOS format.

lib/Sema/SemaCast.cpp

2316

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

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

2318

Indent.

2319

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

2321

There is a CK_ZeroToOCLEvent, why not use that?

2324–2328

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

[pxli168]

lib/Sema/SemaCast.cpp

2324–2326

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

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

2322–2323

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

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

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

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!

include/clang/Basic/DiagnosticSemaKinds.td

   "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<
   "program scope variable must reside in %0 address space">;
 def err_missing_actual_pipe_type : Error<

lib/Sema/SemaCast.cpp

       return;
     }
 
+    // OpenCL v2.0 s6.13.10 - Allow casts from '0' to event_t type.
+    if (Self.getLangOpts().OpenCL && DestType->isEventT()) {
+      llvm::APSInt intValue;
+      if (SrcExpr.get()->EvaluateAsInt(intValue, Self.Context)) {
+        if (0 == intValue) {
+          Kind = CK_ZeroToOCLEvent;
+          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();

test/CodeGenOpenCL/event_t.cl

 // 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)
 }

test/SemaOpenCL/event_t.cl

   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 'int' to 'event_t'}}
 }