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

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In https://reviews.llvm.org/D47553#1117786, @jfb wrote:

> In https://reviews.llvm.org/D47553#1117763, @asb wrote:
>
> > 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 😉


I have a lot of sympathy for @jyknight's suggestion on the GCC bug thread - that if RISC-V wants to offer different guarantees for atomic libcalls vs other platforms it should use a different name for them. I haven't worked with these libcalls before and haven't followed the history, so I lack the credentials to form a strong opinion. If you have a strong feeling one way or the other, please do speak up in the GCC bug thread.

If GCC is just doing the wrong thing then obviously we shouldn't blindly copy. However, _if_ it is reasonable for RISC-V to specify a platform/ABI constraint that `__atomic_*` libcalls <= native register size are lock-free for -march=rv32ia, then it would be a shame if LLVM were unable to exploit that fact.


https://reviews.llvm.org/D47553