[PATCH] D104915: [OpenCL] Add support of __opencl_c_read_write_images feature macro

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10106
 def err_opencl_invalid_read_write : Error<
-  "access qualifier %0 can not be used for %1 %select{|prior to OpenCL version 2.0}2">;
+  "access qualifier %0 can not be used for %1 %select{|prior to OpenCL C version 2.0 or in 3.0 and without __opencl_c_read_write_images feature}2">;
 def err_opencl_multiple_access_qualifiers : Error<
----------------
Suggesting a slight rewording:
`prior to OpenCL C version 2.0 or in version 3.0 without __opencl_c_read_write_images`


================
Comment at: clang/lib/Basic/Targets.cpp:746
 
   // Validate that feature macros are set properly for OpenCL C 3.0.
   // In other cases assume that target is always valid.
----------------
Maybe we should change this comment to something like:

`// Validate the feature dependencies for OpenCL C 3.0.`

Since it is not exactly about the macros.


================
Comment at: clang/lib/Basic/Targets.cpp:752
+  bool IsValid = true;
+  // Some features are dependent on other features
+  for (auto &FeaturePair :
----------------
Maybe we should comment that the first element of the pair is the feature that depends on the feature in the second element?


================
Comment at: clang/lib/Basic/Targets.cpp:757
+        !hasFeatureEnabled(OpenCLFeaturesMap, FeaturePair.second)) {
+      IsValid &= false;
+      Diags.Report(diag::err_opencl_feature_requires)
----------------
I guess we don't need the compound assignment here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104915