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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added a comment.

In D150985#4369279 <https://reviews.llvm.org/D150985#4369279>, @tra wrote:

> 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.

will add test for c11 variants.



================
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}}
----------------
tra wrote:
> 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.
you are right. here should emit a diag "must be a pointer to integer or supported floating point type". will fix.


================
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);
 
----------------
tra wrote:
> We probably want to add tests for `double`, too.
will do


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

https://reviews.llvm.org/D150985