[PATCH] D150985: [clang] Allow fp in atomic fetch max/min builtins
Yaxun Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 26 11:07:55 PDT 2023
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
More information about the cfe-commits
mailing list