[PATCH] D130621: [RISCV] Add target feature to force-enable atomics

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 2 05:49:46 PDT 2022


nikic added a comment.

In D130621#3690619 <https://reviews.llvm.org/D130621#3690619>, @asb wrote:

> Broadly speaking, this looks good to me (thanks!). I think in some previous discussions on this topic I hadn't appreciated the lock-free nature of the __sync calls vs __atomic.
>
> My one request would be to flesh out the test coverage so the files cover all of the different atomicrmw operations - given they all had to be explicitly listed in the `setOperationAction` call to avoid their expansion, it would be worth verifying this (to provide some resilience against future refactorings).

Good call -- my implementation actually failed to handle the atomicrmw float operations -- those always need IR expansion.


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

https://reviews.llvm.org/D130621



More information about the llvm-commits mailing list