[PATCH] D22073: libc++: test lock-free atomic alignment

[Eric Fiselier via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D22073#482120, @jfb wrote:

> In https://reviews.llvm.org/D22073#481402, @EricWF wrote:
>
> > - The test should be moved to `test/libcxx/atomics/atomics.align` since it's libc++ specific.
>
>
> Done.
>
> > - Please give the anonymous struct declarations unique names. `T1`, `T2`, ..., `TN` is fine. Currently they all mangle to `main::type` in diagnostic output.
>
>
> Done. I'll update test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp in a separate change to do the same (I wrote that one as well, didn't name the structs at the time).
>
> > - The test fails to link on my machine unless I manually links `-latomic`. The tests currently don't link `-latomic` and I don't want to turn it on by default. I'll try and fix this.
>
>
> OK, LMK if I can do anything. I build with:
>
>   cmake -G Ninja ../llvm \
>     -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
>     -DLLVM_BUILD_TESTS=ON \
>     -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>     -DLLVM_ENABLE_ASSERTIONS=ON \
>     -DCMAKE_C_COMPILER=/path/to/bin/clang \
>     -DCMAKE_CXX_COMPILER=/path/to/bin/clang++ \
>     -DLLVM_PATH=/path/to/llvm \
>     -DLIBCXX_CXX_ABI=libcxxabi \
>     -DLIBCXX_CXX_ABI_INCLUDE_PATHS=/path/to/llvm/projects/libcxxabi/include \
>     -DLIBCXX_HAS_ATOMIC_LIB=True
>
>
> I found it weird to have to specify `-DLIBCXX_HAS_ATOMIC_LIB=True` because it tells me that libc++ isn't tested *at all* with GCCMM atomic runtime functions <https://gcc.gnu.org/wiki/Atomic/GCCMM>! I was going to look into that afterwards.


Right, but I'm not concerned by that. Our `<atomic>` implementation is a very shallow wrapper over compiler builtins and I would expect the compiler to test those builtins properly.
What do you expect libc++ to gain from testing this?

> IMO we may want to always link to it.


IMHO we don't want to link it unless we need it internally. By not linking it the tests prove that all internally used atomic operations are lock free.  I don't see what we gain from linking it unconditionally, except for maybe ease of use when users have "oversized" types.


https://reviews.llvm.org/D22073