This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Rename lang mode flag for C++ mode
ClosedPublic

Authored by Anastasia on Jul 22 2019, 10:07 AM.

Details

Summary

This suggestion was made in https://reviews.llvm.org/D64418?id=210101#inline-579453 but I am changing this standalone here.

I see two benefits for this rename:

  • Aligning better with OpenCL C conversion
  • Removing ambiguity with OpenCL C++

Since this is the first official release that contains C++ for OpenCL support it seems logical to change at this stage.

Diff Detail

Repository
rL LLVM

Event Timeline

Anastasia created this revision.Jul 22 2019, 10:07 AM
kpet added a comment.Jul 23 2019, 3:10 AM

Thanks for doing this!

include/clang/Frontend/LangStandards.def
177 ↗(On Diff #211134)

Shouldn't this be openclcpp?

Anastasia updated this revision to Diff 211315.Jul 23 2019, 9:23 AM
  • Fixed LANGSTANDARD_ALIAS_DEPR
Anastasia marked an inline comment as done.Jul 23 2019, 9:24 AM
Anastasia added inline comments.
include/clang/Frontend/LangStandards.def
177 ↗(On Diff #211134)

Definitely! Good spot! Thanks!

kpet added a comment.Jul 23 2019, 9:35 AM

Hmm, maybe we need to make sure that one of the tests is using a C++ feature and building with CLC++. This would have caught the mistake.

Anastasia updated this revision to Diff 211320.Jul 23 2019, 9:45 AM

Use -cl-std=CLC++ spelling for test/CodeGenOpenCLCXX/addrspace-with-class.cl

Hmm, maybe we need to make sure that one of the tests is using a C++ feature and building with CLC++. This would have caught the mistake.

That's right! The only test we had wasn't using any C++ features in it. :)

svenvh accepted this revision.Jul 24 2019, 3:07 AM

LGTM!

include/clang/Frontend/LangStandards.def
177 ↗(On Diff #211320)

It surprised me that the capitalized aliases for the (non-C++) CL standards are considered deprecated, because the OpenCL spec only mentions the capitalized variants. The deprecation was introduced by 59456511f4 ("Improve diagnostics for bad -std= flag.", 2017-04-27). It doesn't seem to have any practical implications though.

The convention for non-OpenCL standards seems to be all lowercase though (e.g. c99, c++14, so it sounds reasonable to mark the capitalized CLC++ as deprecated.

This revision is now accepted and ready to land.Jul 24 2019, 3:07 AM
Anastasia marked an inline comment as done.Jul 24 2019, 3:51 AM
Anastasia added inline comments.
include/clang/Frontend/LangStandards.def
177 ↗(On Diff #211320)

Yeah, a misalignment indeed.

kpet accepted this revision.Jul 24 2019, 3:53 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 25 2019, 4:03 AM