[PATCH] D101087: [OpenCL] Introduce new method for validating OpenCL target

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:365
+def warn_opencl_unsupported_core_feature : Warning<
+  "%0 is a core feature in OpenCL C %1 but not supported on this target">,
+  InGroup<OpenCLCoreFeaturesDiagGroup>, DefaultIgnore;
----------------
I would prefer:

`OpenCL C` -> `OpenCL version`

then it should cover better C++ for OpenCL as it is aligned with OpenCL versions. This is also more consistent with how we report most of the diagnostics with language versions.

But in the future, I want to refactor the diagnostics to have wording about OpenCL C and C++ for OpenCL separately, like we already do in `err_opencl_unknown_type_specifier` that uses the following helper https://clang.llvm.org/doxygen/classclang_1_1LangOptions.html#a730c1c883221b87b7c3aaec4327b814b

We definitely need a separate pass over diagnostic wording and how we report OpenCL language versions. 


================
Comment at: clang/include/clang/Basic/OpenCLOptions.h:179
+  static bool isOpenCLOptionCoreIn(const LangOptions &LO, Args &&... args) {
+    return OpenCLOptionInfo(std::forward<Args>(args)...).isCoreIn(LO);
+  }
----------------
Do you think we could avoid constructing the temporary objects somehow?

I.e. could we just check the condition `CLVer >= Avail` that is used in the non-static member function directly?

We could then use these helpers in the non-static members perhaps to avoid duplication.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101087/new/

https://reviews.llvm.org/D101087