[embedded] Make embedded swift_once thread-safe #69498
Conversation
|
@swift-ci please test |
| } | ||
|
|
||
| fileprivate func compareExchangeRelaxed(_ atomic: UnsafeMutablePointer<Int>, expectedOldValue: Int, desiredNewValue: Int) -> Bool { |
There was a problem hiding this comment.
we need to be careful with these: Cortex-M 0 don't have a lot when it comes to atomics. From my understanding all they have are DMB and register based spin locks (which the spin locks are not accessible by compiler intrinsics, and require library level access to those).
There was a problem hiding this comment.
Good point. Though these atomic builtins are valid LLVM builtins for any target and it's the LLVM backend's job to lower them to something that works, worst case a library call. I think that we should (1) keep using atomic builtins here in the embedded runtime code and rely on compiler-rt or a user-provided library to make them actually work, and if the user knows that the system is single-threaded, those can be implemented in a trivial way, and (2) eventually provide an option to use an (alternative) single-threaded embedded runtime which doesn't do any of the atomic stuff (and saves a little bit of codesize on that).
| static var n = 0 | ||
|
|
||
| init() { | ||
| print("MyStructWithAnExpensiveInitializer.init") |
There was a problem hiding this comment.
it might be good to have a recursive initializer test, or dependent ones.
There was a problem hiding this comment.
Added a test for dependent initializers. Having recursion / a cycle in dependent initializers is not valid Swift, correct?
There was a problem hiding this comment.
Right:
<stdin>:1:1: error: circular reference
let a = a
If you manage to hide it from the compiler, I believe you get a runtime error.
| // TODO/FIXME: The following only works in single-threaded environments. | ||
| if predicate.pointee == 0 { | ||
| predicate.pointee = 1 | ||
| if loadAcquire(predicate) < 0 { return } |
There was a problem hiding this comment.
If the only valid values are -1, 0, and 1, then it might be good to raise a fatal error for other values, at least in debug builds.
There was a problem hiding this comment.
I'll be interested to see how the AVR back end handles this! I'm sort of assuming atomics are a noop in AVR! (all single core machines)
There was a problem hiding this comment.
AVR
I assume LLVM will lower all these into library calls, something like __atomic_load, __atomic_store, __atomic_compare_exchange, and expect that you either link compiler-rt that implements them, or that you provide your own definitions of them.
Though single-core doesn't necessarily mean single-threaded, but I assume all sane AVR setups are actually single-threaded?
There was a problem hiding this comment.
Yeah. Someone might contradict me but I'd say the only possible preemption on an AVR CPU is interrupt handlers.
I think we'll see how it lowers if and when we link to the new embedded runtime. And if the AVR backend doesn't have atomics it'll be an obvious compiler crash and an easy patch.
…s dependending on other lazy initializers
|
@swift-ci please test |
In the embedded runtime, replace the current implementation of swift_once that doesn't attempt to handle multithreading at all, with a version that uses atomics on the predicate + spinning on concurrent calls (which is still far from perfect, but for now let's avoid introducing a dependency on lock APIs).