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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I guess targets like SPIR will be supporting all features by default?

At this point, I would prefer that we only add the features that affect the parsing directly i.e. used in the clang source code.

FYI I think we need to explain the common design of features and extensions in comments and some code renaming somehow. But I am not clear yet and especially that it is likely some more refactoring will take place. Let's think about this in the meantime.



================
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)
----------------
Does this need to be in the frontend?


================
Comment at: clang/include/clang/Basic/OpenCLExtensions.def:121
+OPENCLFEAT_INTERNAL(__opencl_c_fp64, 120, ~0U)
+OPENCLFEAT_INTERNAL(__opencl_c_int64, 100, ~0U)
+OPENCLFEAT_INTERNAL(__opencl_c_images, 100, ~0U)
----------------
if we are not going to change clang to make int64 conditional I would suggest we don't add this here for now.


================
Comment at: clang/include/clang/Basic/OpenCLOptions.h:105
     }
     OptMap[Ext].Supported = V;
   }
----------------
I guess we need to make sure that targets can't conditionally support features in OpenCL 2.0 or earlier standards.


================
Comment at: clang/lib/Frontend/CompilerInstance.cpp:950
+  if (getLangOpts().OpenCL)
+    getTarget().getSupportedOpenCLOpts().adjustFeatures(getLangOpts());
+
----------------
Would it be possible to move this into `getTarget().adjust(getLangOpts())` just below. There is a FIXME that explains that we should be doing such adjustment differently but we haven't solved it with time so let's keep the flow as is for now. 


================
Comment at: clang/test/Preprocessor/opencl-feature-extension-simult.cl:15
+
+// RUN: %clang_cc1 %s -E -cl-std=CL3.0 -cl-ext=-all__opencl_c_fp64
+// RUN: %clang_cc1 %s -E -cl-std=CL3.0 -cl-ext=+__opencl_c_fp64
----------------
Is this a typo?

`all__opencl_c_fp64`


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

https://reviews.llvm.org/D89869