[libc++] [C++2b] [P0943] Add stdatomic.h header.
Needs ReviewPublic
Actions

Authored by curdeius on Feb 19 2021, 4:54 AM.

Details

Reviewers

ldionne
__simt__
jfb

Group Reviewers

Restricted Project

Summary

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

curdeius created this revision.Feb 19 2021, 4:54 AM

Herald added a reviewer: jfb. · View Herald TranscriptFeb 19 2021, 4:54 AM

Herald added subscribers: jfb, arichardson. · View Herald Transcript

curdeius requested review of this revision.Feb 19 2021, 4:54 AM

Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 4:54 AM

Herald added a reviewer: Restricted Project. · View Herald Transcript

Herald added a subscriber: libcxx-commits. · View Herald Transcript

curdeius edited the summary of this revision. (Show Details)Feb 19 2021, 4:55 AM

curdeius edited the summary of this revision. (Show Details)

Fix C++ includes in C.

Harbormaster completed remote builds in B89910: Diff 324954.Feb 19 2021, 6:21 AM

Harbormaster completed remote builds in B89909: Diff 324953.

Guard stdatomic.h version test on libcpp-has-no-threads.

Mordante added a subscriber: Mordante.Feb 19 2021, 9:12 AM

Mordante added inline comments.

libcxx/include/stdatomic.h
2	Copy-paste of wchar.h.

curdeius added inline comments.Feb 19 2021, 9:14 AM

libcxx/include/stdatomic.h
2	Thanks for pointing this. I'll fix it in the next revision.

Does it line up property with ./clang/lib/Headers/stdatomic.h ? I think this other header might need some adjustment as well, for example the hosted/freestanding part.

Harbormaster completed remote builds in B89933: Diff 325006.Feb 19 2021, 12:15 PM

I haven't found anything really different from https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdatomic.h.
There are some differences because e.g. ATOMIC_VAR_INIT, atomic_init, kill_dependency aren't mentioned in the paper, but nothing crucial IMO.
Concerning hosted/freestanding, I'm not sure I know what should be verified/done. The clang/lib/Headers/stdatomic.h just includes system's stdatomic.h in hosted env. when available.

What I am more afraid of is rather incompatibilities between C's _Atomic and C++'s atomic.
I haven't tested it extensively, but there were at least some differences in sizeof. And the standard (strongly) recommends to make them compatible (cf. https://eel.is/c++draft/stdatomic.h.syn#4).

libcxx/include/stdatomic.h
133	Should I use `::_VSTD::atomic<T>` or should we go for changing `_VSTD` to `::std`? Or is it not worth the trouble?
libcxx/test/std/atomics/stdatomic.h.syn/types.pass.cpp
21	Anyone has an idea about this? Is this an omission in the standard (given that it includes `atomic_char8_t`) or it should match C11 and so omit both `ATOMIC_CHAR8_T_LOCK_FREE` and `atomic_char8_t`?

curdeius retitled this revision from [libc++] [c++2b] [P0943] Add stdatomic.h header. to [libc++] [C++2b] [P0943] Add stdatomic.h header..Feb 24 2021, 1:49 AM