[PATCH] D47553: Add TargetLowering::shouldExpandAtomicToLibCall and query it from AtomicExpandPass

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

In https://reviews.llvm.org/D47553#1117763, @asb wrote:

> In https://reviews.llvm.org/D47553#1117753, @efriedma wrote:
>
> > On other architectures which have 32-bit atomics, but not 8 and 16-bit atomics, it's generally possible to lower smaller operations using a 32-bit cmpxchg loop.  Does that not work on RISCV for some reason?
>
>
> We can absolutely lower to ll+sc, which is what the 8 and 16-bit libatomic implementations do for rv32ia. My thought was simply:
>
> 1. Simply generating the libcall is easy and correct. This gives a stepping stone allowing the straight-forward implementation of initial support that can then be improved by switching to generating in-line instruction sequences
> 2. If we are happy that the 8 and 16bit __atomic_* libcalls are lock-free, it is safe to choose between inline instruction sequences and libcalls freely. In that case, a hook such as this would allow you to use the libcall when optimising for code size.
>
>   I could understand an argument that use-case 1) isn't compelling enough to add this new hook, and determining whether use-case 2) makes sense is dependent on the conclusion of the GCC bug 86005 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86005> discussion. Thanks @jyknight for sharing your thoughts in that GCC thread - much appreciated.


I think you want the ll/sc code for smaller atomics, it's an assumption on many platforms that you have lock-free 8 / 16. / 32 bit atomics, and often double-pointer-wide atomics too. I don't think your libcalls will generally be lock-free.  I'd be worried if, even for bringup, your platform at one point in time didn't have that property, because then it sets a precedent for people saying "oh but this one time RISCV didn't do *blah*" and then we'll have endless debates about silly platforms. Please avoid me the debates 😉


https://reviews.llvm.org/D47553