This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [FreeBSD] only use _umtx_op(2) on 64bit arches
ClosedPublic

Authored by kib on Jan 23 2023, 4:58 PM.

Details

Summary
Only 64bit architectures can be supported this way, because libcxx
defines __cxx_contention_t to be int64_t for FreeBSD, and 32bit
arches do not have a kind of UMTX_OP_WAIT_INT64_PRIVATE operation.

Fixes: 83387dbc18e7998f87aa4a2d35320bcb2ed5c392

Diff Detail

Event Timeline

kib created this revision.Jan 23 2023, 4:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2023, 4:58 PM
kib requested review of this revision.Jan 23 2023, 4:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2023, 4:58 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
emaste accepted this revision.Jan 23 2023, 5:25 PM
kib edited the summary of this revision. (Show Details)Jan 23 2023, 7:59 PM

Using __LP64__ means we won't use umtx for CHERI architectures. Can we change this to __SIZEOF_LONG__ == 8?

kib updated this revision to Diff 491673.Jan 24 2023, 2:08 AM

use SIZEOF_LONG == 8 instead of LP64 for CHERI.

arichardson accepted this revision.Jan 24 2023, 10:13 AM

use SIZEOF_LONG == 8 instead of LP64 for CHERI.

Thanks!

Thanks for working on this!

libcxx/src/atomic.cpp
80–85

Does this still work on FreeBSD platforms where __SIZEOF_LONG__ != 8 ?

Does this still work on FreeBSD platforms where SIZEOF_LONG != 8 ?

This switches the __SIZEOF_LONG__ != 8 platforms back to the timed backoff fallback. This isn't great, but also relatively low priority, they're all Tier-2 platforms for us.

Mordante accepted this revision.Jan 25 2023, 10:04 AM

Does this still work on FreeBSD platforms where SIZEOF_LONG != 8 ?

This switches the __SIZEOF_LONG__ != 8 platforms back to the timed backoff fallback. This isn't great, but also relatively low priority, they're all Tier-2 platforms for us.

Can you this information as comment in the code?
Otherwise LGTM from libc++'s perspective.

I assume you want to backport this to the release branch, right?

This revision is now accepted and ready to land.Jan 25 2023, 10:04 AM
kib updated this revision to Diff 495703.Feb 7 2023, 6:10 PM

Add comment explaining why umtx is only used on 64bit arches.

arichardson accepted this revision.Feb 8 2023, 1:09 AM
ldionne accepted this revision.Feb 10 2023, 11:08 AM
ldionne added a subscriber: ldionne.

LGTM if this fixes FreeBSD.

This revision was landed with ongoing or failed builds.Mar 1 2023, 12:52 PM
This revision was automatically updated to reflect the committed changes.

@emaste it looks like you landed an older diff?