Page MenuHomePhabricator

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

Authored by ashi1 on Feb 24 2016, 12:04 PM.

Diff Detail


Event Timeline

ashi1 updated this revision to Diff 48966.Feb 24 2016, 12:04 PM
ashi1 retitled this revision from to [OpenCL]Allowing explicit conversion of "0" to event_t type.
ashi1 updated this object.
ashi1 added reviewers: Anastasia, pxli168, yaxunl.
pxli168 edited edge metadata.Feb 24 2016, 10:42 PM

Is above use have been hint by some test cases?

7679 ↗(On Diff #48966)

Please do not use DOS format.

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)


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)

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

pxli168 added inline comments.Feb 25 2016, 12:43 AM
2324–2326 ↗(On Diff #48966)

Need a test case for this.

Anastasia edited edge metadata.Feb 26 2016, 2:29 AM

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

ashi1 marked 7 inline comments as done.Feb 26 2016, 8:37 AM
ashi1 added a subscriber: ashi1.

Joey Gouly or Joey Korkames?


ashi1 edited subscribers, added: Anastasia; removed: ashi1.

Joey Gouly!


ashi1 added a reviewer: joey.Feb 26 2016, 8:50 AM
ashi1 updated this revision to Diff 49219.Feb 26 2016, 12:13 PM
ashi1 marked an inline comment as done.

Revised with Xiuli Pan's comments.

majnemer added inline comments.
2317 ↗(On Diff #49219)

A similar patch has been submitted and rejected several times:

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 added inline comments.Feb 26 2016, 1:13 PM
2317 ↗(On Diff #49219)

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

I will keep you updated.

yaxunl added inline comments.May 5 2016, 1:51 PM
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 marked 5 inline comments as done.May 9 2016, 11:03 AM
ashi1 updated this revision to Diff 56599.May 9 2016, 11:07 AM

Added changes for comments from majnemer.

ashi1 updated this revision to Diff 56615.May 9 2016, 1:12 PM

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

joey accepted this revision.May 11 2016, 1:36 AM
joey edited edge metadata.


This revision is now accepted and ready to land.May 11 2016, 1:36 AM
yaxunl accepted this revision.May 11 2016, 8:11 AM
yaxunl edited edge metadata.

LGTM. Thanks!

Xiuli, are you OK with this patch?

pxli168 accepted this revision.May 19 2016, 9:49 PM
pxli168 edited edge metadata.

Sorry about late reply.

This revision was automatically updated to reflect the committed changes.