[PATCH] D89869: [OpenCL] Define OpenCL feature macros for all versions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/include/clang/Basic/OpenCLExtensions.def:110
+OPENCLFEAT_INTERNAL(__opencl_c_generic_address_space, 200, ~0U)
+OPENCLFEAT_INTERNAL(__opencl_c_work_group_collective_functions, 200, ~0U)
+OPENCLFEAT_INTERNAL(__opencl_c_atomic_order_acq_rel, 200, ~0U)
----------------
azabaznov wrote:
> Anastasia wrote:
> > Does this need to be in the frontend?
> I can remove features which affect only header from this file. But in this case we need to extend '-cl-ext' to register unknown features/extensions (http://lists.llvm.org/pipermail/cfe-dev/2020-October/066932.html). I think this option functionality extending should be done in a separate commit, so we can keep this kind of features here at least for now. What do you think?
Yes, absolutely let's focus on frontend only features for now. I hope we will clarify what we do with the rest in the next few weeks.


================
Comment at: clang/include/clang/Basic/OpenCLOptions.h:105
     }
     OptMap[Ext].Supported = V;
   }
----------------
azabaznov wrote:
> Anastasia wrote:
> > I guess we need to make sure that targets can't conditionally support features in OpenCL 2.0 or earlier standards.
> This can be done by adding a new method //TargetInfo::setSupportedOpenCL30Features()// which can be called in //::adjust//, does this make sense? I believe now current  options setting (//TargetInfo::setSupportedOpenCLOpts()//) knows nothing about OpenCL version which.
I would prefer to use one method for both. We could check the OpenCL version before invoking this or extend it with LangOpts param? Although on second thought since we are going this route of adding features into earlier standards we could just allow targets to configure in a way they like. i.e. with no extra checking per version allowing more flexibility if they don't need to worry about being stricktly conformant... as soon as it doesn't generate an integrity issue for old standards...


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

https://reviews.llvm.org/D89869