This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL][PR40603] In C++ preserve backwards compatibility with OpenCL C v2.0
ClosedPublic

Authored by Anastasia on Feb 6 2019, 8:47 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Anastasia created this revision.Feb 6 2019, 8:47 AM
svenvh added inline comments.Feb 7 2019, 2:56 AM
lib/Frontend/InitPreprocessor.cpp
1063 ↗(On Diff #185564)

Why not set OpenCLVersion in lib/Frontend/CompilerInvocation.cpp instead? Then you wouldn't have to "override" the version in multiple places (which increases the risk of missing one or more places).

lib/Sema/Sema.cpp
265 ↗(On Diff #185564)

This also exposes the OpenCL 2.0 types in OpenCL C++ mode; it would be good to mention that in the commit message.

Anastasia marked an inline comment as done.Feb 7 2019, 3:39 AM
Anastasia added inline comments.
lib/Frontend/InitPreprocessor.cpp
1063 ↗(On Diff #185564)

I think we deliberately wanted to separate OpenCL C versions from OpenCL C++. One big reason is that we don't automatically inherit the same behavior but rather do it explicitly. I could look at moving this logic into extensions implementation directly however this will mean it will need to know more internals of LangOpts. I will give it a try!

Anastasia updated this revision to Diff 185774.Feb 7 2019, 8:05 AM
Anastasia retitled this revision from [OpenCL][PR40603] Align the use of extensions in C++ to be backwards compatible with OpenCL C v2.0 to [OpenCL][PR40603] In C++ preserve backwards compatibility with OpenCL C v2.0.
Anastasia edited the summary of this revision. (Show Details)
  • Moved version check inside the extension logic.
  • Changed commit message to reflect better functionality of the patch.
svenvh accepted this revision.Feb 7 2019, 8:12 AM

Even nicer to have it only inside the extension logic indeed.

LGTM.

This revision is now accepted and ready to land.Feb 7 2019, 8:12 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 9:32 AM