[PATCH] D120026: [ARM] Fix ARM backend to correctly use atomic expansion routines.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I think we're going to need an option to restore the previous behavior for Rust. The context here is that Rust targets can separately specify up to which size the support atomic load/store, and whether they support atomic CAS. Historically, the thumbv6m target allowed atomic load/store and disabled CAS, relying on the previous LLVM behavior of emitting atomic load/store as a memory barrier + simple load/store.

Of course, this makes Rust code using atomics incompatible with any C code using atomic CAS via libatomic (or I guess any other CAS implementation without OS support). There is an implicit assumption here that Rust code for this target will never get linked against C code using libatomic. I think this assumption was made by accident, but it's probably a fairly reasonable assumption to make for common use-cases, especially for baremetal targets.

Based on the feedback I received, removing support for atomic load/store for these targets would be a major breaking change for the embedded ecosystem. The other bit of context here is that Rust atomics are required to be lock-free -- I guess the alternative would be to go back on that and actually use libatomic, but that would probably come with its own complications, especially for this kind of target.

My thinking here is that we could add a target feature which basically says "I promise I won't use atomic CAS", in which case atomic load/store can be correctly lowered as before. Does that sound reasonable? It's worth noting that there is also interest in something like this for other targets, such as riscv without the A extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120026