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