This is an archive of the discontinued LLVM Phabricator instance.

Introduce a cmake module to figure out whether we need to link with libatomic.
ClosedPublic

Authored by vkalintiris on Jan 26 2016, 4:42 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vkalintiris retitled this revision from to Use LLVM's CheckAtomic cmake module to figure out whether we need to link with libatomic..
vkalintiris updated this object.
vkalintiris added a subscriber: cfe-commits.
jroelofs accepted this revision.Jan 26 2016, 4:52 PM
jroelofs edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 26 2016, 4:52 PM
vkalintiris edited edge metadata.

Don't use LLVM's CheckAtomic cmake module. Instead, use a libcxx-specific one
that allows checking for 64-bit atomics too.

vkalintiris retitled this revision from Use LLVM's CheckAtomic cmake module to figure out whether we need to link with libatomic. to Introduce a cmake module to figure out whether we need to link with libatomic..Jan 28 2016, 7:26 AM
vkalintiris added a reviewer: joerg.
joerg edited edge metadata.Jan 28 2016, 8:29 AM

Thanks for working on it. I think it might be slightly better to explicitly test uintmax_t and uintptr_t as well. They could be larger than long long int and there are tests depending on the existance.

Do we have any test cases for arbitrary sized trivally copyable structures? That might also be needed.

vkalintiris edited edge metadata.

Check for atomcis on uintmax_t/uintptr_t instead of long long int.

Is this okay to commit with the updated changes?

hans added a subscriber: hans.Jan 29 2016, 1:27 PM
dsanders edited edge metadata.Feb 3 2016, 1:54 AM

FWIW, the changes in the last revision look minor to me. I doubt it affects the LGTM

Do we have any test cases for arbitrary sized trivally copyable structures? That might also be needed.

The failures in 3.8.0rc1 (and presumably still occur in the 3.8.0rc2 that was tagged last night) were all related to 64-bit atomics.

vkalintiris added a comment.EditedFeb 5 2016, 4:32 PM

Sorry for the early pings. In case it's not clear from Daniel's comment, without this patch libc++ will be broken for 32-bit MIPS CPUs. The relevant bug report has been nominated as a release blocker but we'd like to have it committed before the next RC.

dsanders accepted this revision.Feb 9 2016, 1:38 AM
dsanders edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.