This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Change default standard version to CL1.2
ClosedPublic

Authored by Anastasia on Jul 21 2021, 3:32 PM.

Details

Summary

As suggested in RFC: https://lists.llvm.org/pipermail/cfe-dev/2021-June/068318.html set default version OpenCL C to 1.2. This means that the absence of any standard flag will be equivalent to passing -cl-std=CL1.2.

Note that this patch also fixes incorrect version check for the pointer to pointer kernel arguments.

Diff Detail

Event Timeline

Anastasia created this revision.Jul 21 2021, 3:32 PM
Anastasia requested review of this revision.Jul 21 2021, 3:32 PM

LGTM, just a small question.

clang/test/CodeGenOpenCL/spir_version.cl
2

Would it be worth having an invocation without any -cl-std= and verifying that it produces the same version metadata as CL1.2?

Anastasia added inline comments.Jul 22 2021, 4:15 AM
clang/test/CodeGenOpenCL/spir_version.cl
2

From a unit test perspective, I personally think that it is sufficient to test that the default version is CL1.2 separately and then test the expected functionality of CL1.2 separately. Otherwise when we change the default version later we will need to modify all the tests again. I don't see a value in this.

svenvh accepted this revision.Jul 22 2021, 4:37 AM
svenvh added inline comments.
clang/test/CodeGenOpenCL/spir_version.cl
2

Agreed, and clang/test/Preprocessor/predefined-macros.c seems to do that already.

This revision is now accepted and ready to land.Jul 22 2021, 4:37 AM
azabaznov added inline comments.Jul 22 2021, 5:11 AM
clang/test/Parser/opencl-atomics-cl20.cl
7

As OpenCL C version defaults to 1.2, I think LANG_VER_OK should always be defined now (due to run lines). But OpenCL C 1.2 has never been tested in this test... Is it a mistake in current rest?

Anastasia added inline comments.Jul 22 2021, 1:55 PM
clang/test/Parser/opencl-atomics-cl20.cl
7

Yes this is a typo in the test or something i.e. LANG_VER_OK should not be defined for the first run line.

This revision was landed with ongoing or failed builds.Jul 26 2021, 7:04 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2021, 7:04 AM
Herald added a subscriber: ldrumm. · View Herald Transcript