[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

 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">;
+  "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;
     }
 
+    // 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) {
+          Kind = CK_IntegralToPointer;
+          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)
 }