[llvm] [RISCV] Select atomic_{load/store} to pseudos and expand them later (PR #67108)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 20 08:57:47 PDT 2023


asb wrote:

First of all, sorry that this fell of the review queue.

Some thoughts:
* The diff vs what we have today isn't massive, so I don't have a strong aversion to this. Though particularly on the tablegen side, there is a bit more target-specific complexity I think.
* You're right that in general atomic operations are expanded later on, though I wouldn't say it _necessarily_ follows that its better to do all in the same way. Alternatively, you could view the current setup as the RISC-V backend using the target-independent hooks wherever possible for atomics codegen and falling back to late stage expansion via a target-specific codepath only when necessary.
* The docs on atomics in LLVM are I think actually pretty good <https://llvm.org/docs/Atomics.html> and I tried to expand them to properly cover what we do on RISC-V, so that should hopefully help anyone diving it. It's hard for me to judge how much the docs make sense for someone who's never messed with atomics lowering in LLVM though!
* If we go in this direction, please add a doc comment to record the motivation for not using the emitLeadingFence/emitTrailingFence hooks.
* Your second reason does make sense to me, but it would be good to get a second opinion - e.g . at preames @topperc 

https://github.com/llvm/llvm-project/pull/67108


More information about the llvm-commits mailing list