svenvh added a comment.
Thanks for the update! I have a few points to improve the patch.
================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:1107
+// The functionality added by cl_ext_float_atomics extension
+let MinVersion = CL30 in {
+ foreach TypePair =
----------------
Do we really need to guard these additions behind OpenCL 3.0? The spec mentions
> The functionality added by this extension uses the OpenCL C 2.0 atomic syntax
> and hence requires OpenCL 2.0 or newer.
(same applies to the opencl-h.c changes of course)
================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:1112
+ foreach AddressSpaceiKind = [GenericAS, GlobalAS, LocalAS] in {
+ def:
+ Builtin<"atomic_fetch_" #ModOp, [
----------------
The feature macros seem to be missing. See `FuncExtOpenCLCPipes` for an
example how to do that.
================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:1132-1135
+ foreach TypePair =
+ [[AtomicFloat, Float, Float], [AtomicDouble, Double, Double]] in {
+ foreach ModOp = ["max", "min"] in {
+ foreach AddressSpaceiKind = [GenericAS, GlobalAS, LocalAS] in {
----------------
This can be merged into the preceeding `foreach` parts I think?
================
Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:139
+
+ resf1 = atomic_fetch_min(a_float, f1);
+ resf1 = atomic_fetch_max(a_float, f1);
----------------
As mentioned in the comment on lines 13-17, this test is not meant to be
exhaustive. So you don't have to test every overload, checking one or two
builtins should suffice.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106343/new/
https://reviews.llvm.org/D106343
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
