[XCore] Set MaxAtomicSizeInBitsSupported to 0 #74389
Conversation
XCore does not appear to have any support for atomicrmw or cmpxchg. This will result in all atomic operations getting expanded to __atomic_* libcalls via AtomicExpandPass, which matches what Clang already does in the frontend. Additionally, remove the code which handles atomic load/store, as it will no longer be used.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
That's correct that XCore does not have atomicrmw or cmpxchg. It lowered aligned atomic load & store because under its hardware memory model they would be inherently atomic.
To check my understanding: within the max bits, we are expected to support all five atomic instructions. So currently aligned 4-byte atomicrmw (for example) crashes with isel error. But unaligned operations will already go to library. So if atomicrmw etc are not supported, load & store cannot be either.
Just to mention, while looking at this, I noticed that TargetLoweringBase.cpp has the comment that the default will be set to 0 - just wondered if that's still the case.
Correct: if native lockless implementations for all 5 operations aren't available for a given size, then it's required to fallback to the _atomic* library routine for all of them. The library may use a lock-based implementation, and unless all the operations go through the library, it isn't possible to guarantee that e.g. cmpxchg vs store are atomic.
Yes, that was a migration I started, then dropped on the floor for ... a few years, sorry. This is me picking that back up. First step is to fix all in-tree targets explicitly, and will change the default afterwards. |
XCore does not appear to have any support for atomicrmw or cmpxchg.
This will result in all atomic operations getting expanded to _atomic* libcalls via AtomicExpandPass, which matches what Clang already does in the frontend.
Additionally, remove the code which handles atomic load/store, as it will no longer be used.