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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
