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

This revision was automatically updated to reflect the committed changes. yaxunl marked 4 inline comments as done. Closed by commit rG00448a548c4e: [clang] Allow fp in atomic fetch max/min builtins (authored by yaxunl). Herald added a project: clang. Changed prior to commit:

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

yaxunl marked 4 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:6576-6578 if (!ValType->isFloatingType()) return false; + if (!(AllowedType & AOAVT_FP)) tra wrote: > Collapse into a single

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

tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM with few more test nits. Comment at: clang/test/Sema/atomic-ops.c:134 int *I, const int *CI, int **P, float *D, struct S *s1, struct S *s2) {

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

tra added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:6576-6578 if (!ValType->isFloatingType()) return false; + if (!(AllowedType & AOAVT_FP)) Collapse into a single if statement: `if (!(ValType->isFloatingType() &&

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

yaxunl updated this revision to Diff 527034. yaxunl added a comment. rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150985/new/ https://reviews.llvm.org/D150985 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/CodeGen/CGAtomic.cpp

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

yaxunl updated this revision to Diff 526258. yaxunl marked 2 inline comments as done. yaxunl added a comment. revised by Artem's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150985/new/ https://reviews.llvm.org/D150985 Files:

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

yaxunl marked 2 inline comments as done. yaxunl added a comment. In 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

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

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,

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

yaxunl added a comment. In D150985#4357207 , @tra wrote: > The code changes look OK to me. > > Whether allowing FP for clang builtins is OK -- I have no idea, especially > for the c11 ones. hardware atomic fmax/fmin instructions are added because users

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

tra added a comment. The code changes look OK to me. Whether allowing FP for clang builtins is OK -- I have no idea, especially for the c11 ones. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150985/new/ https://reviews.llvm.org/D150985

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

yaxunl created this revision. yaxunl added reviewers: rjmccall, tra. Herald added subscribers: kerbowa, jvesely. Herald added a project: All. yaxunl requested review of this revision. LLVM IR already allows floating point type in atomicrmw. Update clang atomic fetch max/min builtins to accept