[PATCH] D85691: lld: link libatomic if needed for Timer

Mon Aug 10 15:59:30 PDT 2020

jfb added a comment.

In D85691#2208348 <https://reviews.llvm.org/D85691#2208348>, @cuviper wrote:

> In D85691#2208323 <https://reviews.llvm.org/D85691#2208323>, @jfb wrote:
>
>> You're probably better off putting the `std::chrono::nanoseconds::rep` behind a lock if it's not always lock free on all platforms. Some platforms just don't have `libatomic`, so this patch might cause other issues.
>
> Wouldn't such a platform get a fatal error in `llvm/cmake/modules/CheckAtomic.cmake`?

Not sure without digging through cmake-isms... but macOS doesn't have libatomic by default 🙂

More importantly, it seems like `std::chrono::nanoseconds::rep` might be the wrong type to make atomic, or if it is then it should be behind a lock. Using locks implicitly through libatomic jsut hides the lock in the runtime. Or maybe atomics and locks are the wrong solution for D80298 <https://reviews.llvm.org/D80298>, and it should accumulate timers some other way 🤷‍♂️

In any case, I don't think using non-lock-free atomics is what you want here. Linking libatomic seems to be patching the wrong issue.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85691/new/

https://reviews.llvm.org/D85691