Page MenuHomePhabricator

[OpenCL] Add supported OpenCL extensions to target info.
ClosedPublic

Authored by yaxunl on Apr 25 2016, 8:51 AM.

Details

Summary

Add supported OpenCL extensions to target info. It serves as default values to save the users of the burden setting each supported extensions and optional core features in command line.

In the future a command line option may be added to allow enable/disable each supported extension or optional core feature individually.

Diff Detail

Repository
rL LLVM

Event Timeline

yaxunl updated this revision to Diff 54862.Apr 25 2016, 8:51 AM
yaxunl retitled this revision from to [OpenCL] Add supported OpenCL extensions to target info..
yaxunl updated this object.
yaxunl added reviewers: Anastasia, bader.
Anastasia edited edge metadata.Apr 28 2016, 10:20 AM

Could you export a full diff please? There are too many small bits here to review! Thanks!

test/SemaOpenCL/extensions.cl
2 ↗(On Diff #54862)

Could we perhaps have two targets in this test - one that supports the extension and one that doesn't to test the rest of your patch?

yaxunl updated this revision to Diff 55600.Apr 29 2016, 8:17 AM
yaxunl edited edge metadata.

Add defining macro cl_khr_ for supported extensions or optional core features.

Enable supported extensions or optional core features only if it is available in the OpenCL version the program is compiled with.

Add spir triple to tests using extensions.

Generated full diffs.

Anastasia accepted this revision.Apr 29 2016, 12:11 PM
Anastasia edited edge metadata.

LGTM! Apart from small comments that can be addressed directly before committing.

include/clang/Basic/OpenCLExtensions.def
18 ↗(On Diff #55600)

Could you add a brief comment explaining usage of both macros.

test/CodeGenOpenCL/builtins-r600.cl
2 ↗(On Diff #55600)

Just triple is not sufficient here?

test/SemaOpenCL/extensions.cl
14 ↗(On Diff #55600)

I was just wondering whether renaming to "unsupported" instead of "unknown" would make more sense?

This revision is now accepted and ready to land.Apr 29 2016, 12:11 PM
yaxunl updated this revision to Diff 55839.May 2 2016, 10:28 AM
yaxunl edited edge metadata.
yaxunl marked 2 inline comments as done.

Add comments about macros for enumerating extensions.
Improve diagnostics about extensions.

test/CodeGenOpenCL/builtins-r600.cl
2 ↗(On Diff #55600)

This test requires fp64 support. Some GPUs of r600 target does not support fp64. Need to specify a GPU supporting fp64.

test/SemaOpenCL/extensions.cl
14 ↗(On Diff #55600)

Agree.

yaxunl added a comment.May 3 2016, 7:30 AM

Hi Alexey,

Any comments on this patch?

Thanks.

bader added inline comments.May 12 2016, 12:51 PM
include/clang/Basic/OpenCLExtensions.def
64 ↗(On Diff #55839)

Minimum required version should be 120.
cl_khr_spir was available in OpenCL 1.2 as well as cl_khr_depth_images and cl_khr_gl_depth_images.
https://www.khronos.org/registry/cl/specs/opencl-1.2-extensions.pdf

yaxunl updated this revision to Diff 57177.May 13 2016, 7:23 AM

Fixed available OpenCL version for some extensions.

bader accepted this revision.May 13 2016, 7:39 AM
bader edited edge metadata.

LGTM. Thanks!

This revision was automatically updated to reflect the committed changes.

In current trunk, I got build error :

llvm/tools/clang/lib/Basic/Targets.cpp:2090:9: error: 'setSupportedOpenCLOpts' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
 void setSupportedOpenCLOpts() {

Can you revert this change if this error is related with this change ?

yaxunl added a subscriber: yaxunl.May 13 2016, 10:22 AM

Done. Thanks for letting me know.

Sam