Page MenuHomePhabricator

[libc++] Link against libatomic when it is found
ClosedPublic

Authored by ldionne on Jun 4 2020, 12:38 PM.

Details

Reviewers
jfb
__simt__
Group Reviewers
Restricted Project
Commits
rGe0184357fc78: [libc++] Link against libatomic when it is found
Summary

Before this patch, we tried detecting whether small atomics were available
without linking against libatomic. However, that's not really what we want
to know -- instead, we want to know what's required in order to support
atomics fully, which is to link against libatomic when it's provided.

That is both much simpler, and it doesn't suffer the problem that we would
not link against libatomic when small atomics didn't require it, which
lead to non-lockfree atomics never working.

Furthermore, because we understand that some platforms might not want to
(or be able to) ship non-lockfree atomics, we add that notion to the test
suite, independently of a potential extern library.

After this patch, we therefore:
(1) Link against libatomic when it is provided
(2) Independently detect whether non-lockfree atomics are supported in

the test suite, regardless of whether that means we're linking against
an external library or not (which is an implementation detail).

Diff Detail

Event Timeline

ldionne created this revision.Jun 4 2020, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2020, 12:38 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne marked an inline comment as done.
ldionne added a subscriber: phosek.
ldionne added inline comments.
libcxx/cmake/config-ix.cmake
101

@phosek Is this right for Fuchsia?

I'm not an expert in the details but this looks right to me.

When this goes in, I will be adding non-lock-free atomic tests.

These tests would fail today, meaning we have no coverage today.

griwes added a subscriber: griwes.Jun 4 2020, 2:31 PM

FWIW this looks good to me as well.

jfb added inline comments.Jun 4 2020, 3:06 PM
libcxx/test/libcxx/selftest/dsl/dsl.sh.py
105

Here and for the others, int main(int, char**) 😄

libcxx/utils/libcxx/test/dsl.py
71

I think your filenames need to be quoted properly otherwise spaces in filenames will bork stuff (above and below).

libcxx/utils/libcxx/test/features.py
39

int main(int, char**)

ldionne updated this revision to Diff 268770.Jun 5 2020, 6:27 AM
ldionne marked 4 inline comments as done.

Address JF's comments

libcxx/utils/libcxx/test/dsl.py
71

Lit will expand filenames like %t and directories like %T, the assumption is that it will quote it properly for shell evaluation.

ldionne accepted this revision as: Restricted Project.Jun 5 2020, 6:28 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2020, 6:37 AM
This revision was automatically updated to reflect the committed changes.
jfb added a comment.Jun 5 2020, 8:27 AM

Thanks for this!