Page MenuHomePhabricator

[OpenCL] Blocks are allowed to capture arrays in OpenCL 2.0 and higher.
Needs ReviewPublic

Authored by echuraev on Nov 17 2016, 5:44 AM.

Details

Reviewers
Anastasia

Diff Detail

Event Timeline

echuraev updated this revision to Diff 78351.Nov 17 2016, 5:44 AM
echuraev retitled this revision from to [OpenCL] Blocks are allowed to capture arrays in OpenCL 2.0 and higher..
echuraev updated this object.
echuraev added a reviewer: Anastasia.
echuraev added subscribers: cfe-commits, yaxunl, bader.
Anastasia edited edge metadata.Nov 17 2016, 10:22 AM

I have created a bug to Khronos regarding this, but unfortunately I don't see it being progressed yet.
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15659

The problem here is that I am not sure we should deviate from the ObjC implementation because OpenCL blocks are largely taken from Clang ObjC implementation. My issue is in particular that it's not clear what the capture of array would mean and spec should either state it precisely or disallow using this feature at all to avoid costly operations. In ObjC community itself there were multiple interpretation of this in the past: http://lists.llvm.org/pipermail/cfe-dev/2016-March/047849.html

I am not sure we should go ahead with any implementation without further clarifications. I will ping the Khronos bug to see if the documentation can be improved.

I think this issue has been seen in the OpenCL conformance tests, but was fixed later on?

I have created a bug to Khronos regarding this, but unfortunately I don't see it being progressed yet.
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15659

The problem here is that I am not sure we should deviate from the ObjC implementation because OpenCL blocks are largely taken from Clang ObjC implementation. My issue is in particular that it's not clear what the capture of array would mean and spec should either state it precisely or disallow using this feature at all to avoid costly operations. In ObjC community itself there were multiple interpretation of this in the past: http://lists.llvm.org/pipermail/cfe-dev/2016-March/047849.html

I am not sure we should go ahead with any implementation without further clarifications. I will ping the Khronos bug to see if the documentation can be improved.

I think this issue has been seen in the OpenCL conformance tests, but was fixed later on?

Sure. Do you have an access to revision with an update? I will ask to publish it online too.

lib/Sema/SemaExpr.cpp
13481

Can we do this consistently for all OpenCL (Not just v2.0)!

echuraev updated this revision to Diff 94503.Apr 7 2017, 3:18 AM
echuraev marked an inline comment as done.

Yes, I have an access to the new revision and I have read it.

Anastasia added inline comments.Apr 7 2017, 7:35 AM
lib/Sema/SemaExpr.cpp
13479

I think the comment should be updated.

test/SemaOpenCL/blocks_with_array.cl
9 ↗(On Diff #94503)

Does this test anything different from the function foo below?

21 ↗(On Diff #94503)

Can we check there is a memcpy here?

27 ↗(On Diff #94503)

Can we CHECK-NOT memcpy?

30 ↗(On Diff #94503)

Do we need this line as well?