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