Page MenuHomePhabricator

[libc++] Add a 'is-lockfree-runtime-function' lit feature
ClosedPublic

Authored by arichardson on Nov 21 2020, 4:33 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG0f81598cc1f4: [libc++] Add a 'is-lockfree-runtime-function' lit feature
Summary

On macOS 10.14 /usr/lib/system/libcompiler_rt.dylib contains all the
__atomic_load*, etc. functions but does not include the __atomic_is_lock_free
function. The lack of this function causes the non-lockfree-atomics feature
to be set to false even though large atomic operations are actually
supported, it's just the is_lock_free() function that is missing.

This is required so that the !non-lockfree-atomics feature can be used
to XFAIL tests that require runtime library support (D88818).

Diff Detail

Event Timeline

arichardson created this revision.Nov 21 2020, 4:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2020, 4:33 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
arichardson requested review of this revision.Nov 21 2020, 4:33 AM

Correctly skip align.pass.pass.cpp for macOS

arichardson edited the summary of this revision. (Show Details)Nov 25 2020, 6:31 AM

rebase after formatting change

ldionne added inline comments.Nov 27 2020, 11:25 AM
libcxx/test/libcxx/atomics/atomics.align/align.pass.pass.cpp
10

Ok, I think we're getting down to the bottom of my concern now. When is !is-lockfree-runtime-function true, but non-lockfree-atomics not true?

I want to understand in what context those can't be substituted for one another.

10

Never mind, I re-read the review summary and it explains it.

How difficult would it be to implement __atomic_is_lock_free in compiler-rt? I think something like this in atomic.c would work:

int __atomic_is_lock_free(size_t size, void const volatile* ptr) {
#define LOCK_FREE_ACTION(type) return sizeof(type) <= size;
  LOCK_FREE_CASES(ptr);
#undef LOCK_FREE_ACTION
  return 0;
}

However, I'm not familiar with how to add builtins in compiler-rt, and I haven't been able to figure out the details at a glance.

Do I understand correctly that:

  1. When Clang sees __atomic_is_lock_free(args...), it tries to generate code directly for it if it can.
  2. Otherwise, it generates an external call to a function called int __atomic_is_lock_free(unsigned long, void const volatile*);
  3. If a library that defines this function isn't provided, a linker error follows

Is that how these work? We can definitely add a workaround to fix the test suite here, however I'd also like to actually solve the root cause of this issue. It's also something we've had requests about in the past.

How difficult would it be to implement __atomic_is_lock_free in compiler-rt? I think something like this in atomic.c would work:

int __atomic_is_lock_free(size_t size, void const volatile* ptr) {
#define LOCK_FREE_ACTION(type) return sizeof(type) <= size;
  LOCK_FREE_CASES(ptr);
#undef LOCK_FREE_ACTION
  return 0;
}

However, I'm not familiar with how to add builtins in compiler-rt, and I haven't been able to figure out the details at a glance.

Do I understand correctly that:

  1. When Clang sees __atomic_is_lock_free(args...), it tries to generate code directly for it if it can.
  2. Otherwise, it generates an external call to a function called int __atomic_is_lock_free(unsigned long, void const volatile*);
  3. If a library that defines this function isn't provided, a linker error follows

Is that how these work? We can definitely add a workaround to fix the test suite here, however I'd also like to actually solve the root cause of this issue. It's also something we've had requests about in the past.

That matches my understanding and what the GCC documentation says:

Built-in Function: bool __atomic_is_lock_free (size_t size, void *ptr)
This built-in function returns true if objects of size bytes always generate lock-free atomic instructions for the target architecture. If the built-in function is not known to be lock-free, a call is made to a runtime routine named __atomic_is_lock_free.
ptr is an optional pointer to the object that may be used to determine alignment. A value of 0 indicates typical alignment should be used. The compiler may also ignore this parameter.

I've also posted D92302 to implement this in compiler-rt.

ldionne accepted this revision.Dec 1 2020, 8:22 AM

Apart from the TODO comment, LGTM. Ship it with the comment.

libcxx/utils/libcxx/test/features.py
60

Can you add a TODO to remove this once compiler-rt ships atomic_is_lockfree?

This revision is now accepted and ready to land.Dec 1 2020, 8:22 AM

Thanks for reviewing!

libcxx/utils/libcxx/test/features.py
60

Sure, will do. However, it will still be needed for older versions of macOS+FreeBSD that don't include the function, so I guess it will be a while until it can be removed.

This revision was automatically updated to reflect the committed changes.
arichardson marked 3 inline comments as done.