[PATCH] D150985: [clang] Allow fp in atomic fetch max/min builtins

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

As I said, I'm OK with the patch in principle, I just don't know what other factors I may be missing.

Tests seem to be missing for c11 variants of the builtins.



================
Comment at: clang/test/Sema/atomic-ops.c:209
+  __atomic_fetch_min(D, 3, memory_order_seq_cst);
+  __atomic_fetch_max(P, 3, memory_order_seq_cst);
   __atomic_fetch_max(p, 3);                       // expected-error {{too few arguments to function call, expected 3, have 2}}
----------------
Is that intentional that we now allow atomic max on a `int **P` ? My understanding that we were supposed to allow additional FP types only.


================
Comment at: clang/test/SemaOpenCL/atomic-ops.cl:65
+  __opencl_atomic_fetch_min(f, 1, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_fetch_max(f, 1, memory_order_seq_cst, memory_scope_work_group);
 
----------------
We probably want to add tests for `double`, too.


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

https://reviews.llvm.org/D150985