yaxunl added a comment. In D71726#2182667 <https://reviews.llvm.org/D71726#2182667>, @tra wrote:
> LGTM, modulo couple of nits. > > @jyknight are you OK with this? > > In D71726#2179428 <https://reviews.llvm.org/D71726#2179428>, @yaxunl wrote: > >> Make IEEE single and double type as supported for fp atomics in all targets >> by default. This is based on the assumption that AtomicExpandPass or its >> ongoing work is sufficient to support fp atomics for all targets. This is to >> facilitate middle end and backend end development to support fp atomics. >> >> If a target would like to treat single and double fp atomics as unsupported, >> it can override the default behavior in its own TargetInfo. > > Do we have sufficient test coverage on all platforms to make sure we're not > generating something that LLVM can't handle everywhere? > If not, perhaps we should default to unsupported and only enable it for known > working targets. I updated TargetInfo for fp atomic support for common targets. Basically by default fp atomic support is now off. It is enabled only for targets which do not generate lib calls for fp atomics. This is because the availability of lib call depends on platform, so it is up to the Target owners to determine whether the support is available if lib call is needed. For those targets which are able to generate llvm fp atomic instructions, fp atomic support is enabled in clang, and tests are added to cover them. ================ Comment at: clang/lib/CodeGen/CGAtomic.cpp:889-891 + if (MemTy->isFloatingType()) { + ShouldCastToIntPtrTy = false; + } ---------------- tra wrote: > `ShouldCastToIntPtrTy = !MemTy->isFloatingType();` done ================ Comment at: clang/test/Sema/atomic-ops.c:102-103 void f(_Atomic(int) *i, const _Atomic(int) *ci, - _Atomic(int*) *p, _Atomic(float) *d, + _Atomic(int*) *p, _Atomic(float) *d, _Atomic(double) *d2, + _Atomic(long double) *d3, int *I, const int *CI, ---------------- tra wrote: > Rename arguments? d -> f, d2 -> d, d3 -> ld ? done CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71726/new/ https://reviews.llvm.org/D71726 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
