This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Add support of __opencl_c_pipes feature macro.
ClosedPublic

Authored by azabaznov on Jul 24 2021, 8:03 AM.

Details

Summary

'pipe' keyword is introduced in OpenCL C 2.0: so do checks for OpenCL C version while
parsing and then later on check for language options to construct actual pipe. This feature
requires support of __opencl_c_generic_address_space, so diagnostics for that is provided as well.

Diff Detail

Event Timeline

azabaznov created this revision.Jul 24 2021, 8:03 AM
azabaznov requested review of this revision.Jul 24 2021, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2021, 8:03 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Anastasia added inline comments.Jul 26 2021, 7:22 AM
clang/include/clang/Basic/DiagnosticCommonKinds.td
154 ↗(On Diff #361457)

If possible I would prefer to unify the diagnostics. How about something like:

'%2' %select{type qualifier|storage class specifier}3 is unsupported in %select{OpenCL C|C++ for OpenCL}0 version %1

then where we print the diagnostic we can special case OpenCL 3.0 to also print additionally the feature information. FYI we could add the special case for OpenCL 3.0 later on in a separate patch and for now just reuse the diagnostic as is. Then this patch could still go into release 13 although a bit tight but doable.

azabaznov added inline comments.Jul 26 2021, 7:30 AM
clang/include/clang/Basic/DiagnosticCommonKinds.td
154 ↗(On Diff #361457)

This is actually the way how clang provides diagnostic now (https://godbolt.org/z/9P6PWdE5M):

OpenCL C version 3.0 does not support the 'generic' type qualifier

Here I just wanted to unify cases for pipe and generic since they both require OpenCL C 2.0 or later with a feature. Do you think it makes sense?

Anastasia added inline comments.Jul 26 2021, 7:44 AM
clang/include/clang/Basic/DiagnosticCommonKinds.td
154 ↗(On Diff #361457)

I think what you are trying to do makes perfect sense i.e. we should improve the diagnostic for sure. But I think we should avoid duplication with err_opencl_type_specifier_requires like you have done in https://reviews.llvm.org/D106260#change-jk5JS8iu1irq

But improving the diagnostic can perfectly be done as a separate step later on.

azabaznov updated this revision to Diff 362018.Jul 27 2021, 7:05 AM

Preserve existing diagnostic with err_opencl_unknown_type_specifier, fix comments for language option

azabaznov marked 2 inline comments as done.Jul 27 2021, 7:05 AM
Anastasia accepted this revision.Jul 27 2021, 8:26 AM

LGTM! Thanks

This revision is now accepted and ready to land.Jul 27 2021, 8:26 AM
This revision was landed with ongoing or failed builds.Jul 29 2021, 7:28 PM
This revision was automatically updated to reflect the committed changes.

Reverted it due too suspicious failing:

error: 'error' diagnostics expected but not seen: 
  File /home/tcwg-buildslave/worker/clang-armv7-2stage/llvm/clang/test/SemaOpenCL/invalid-pipes-cl2.0.cl Line 8: type '__global write_only pipe int ({{(void)?}})' can only be used as a function parameter in OpenCL
error: 'error' diagnostics seen but not expected: 
  File /home/tcwg-buildslave/worker/clang-armv7-2stage/llvm/clang/test/SemaOpenCL/invalid-pipes-cl2.0.cl Line 8: type '__private write_only pipe int (void)' can only be used as a function parameter in OpenCL
2 errors generated.

I don't think that's because of triple was unset, I think the main reason was that program scope variables were not supported, but need to have a look closely