[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