-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
16 byte cmpxchg #524
Comments
I'm not sure why we'd need anything on Arm. Arm has load-reserved, store-exclusive, so doesn't need a second word for ABA. The only platforms where we have a problem have the following combination of things:
This means that you need the combination of x86, libstdc++, and gcc to see the problem. GCC has an open bug report about it and so, presumably, will fix it eventually. That said, the ABA code isn't used on any hot paths in snmalloc, so the cost of a libcall is unlikely to be noticeable. This combination of things (bug in a single compiler, doesn't affect hot paths) means that I'm very hesitant to do something ad-hoc as a work around for that compiler. Looking at the code, we seem not to be doing LL/SC for non-x86 architectures. I thought @nwf added that a while ago? |
I had a look at LL/SC on Arm, but couldn't work out how to do it. The ABA primitive is designed to be able to use it, I just didn't know a good way to actually achieve it without complex inline assembly across functions. The use of the ABA protection is only for allocating allocators, and potentially in the backend. So falling back to locks for ABA protection is actually fine in most cases. |
Hi, I initiated this issue because I found that LLVM and GCC would behave quite differently on whether to inline atomic ops. This may deliver extra linkage with libatomic.so for rust. For example, on x86, we are requiring the compiled target to have |
For DW LL/SC, maybe https://github.com/taiki-e/portable-atomic/blob/main/src/imp/aarch64.rs can be considered as a reference? |
I think there are two points to improve in current cmpxchg support for 16 byte data structures:
__sync_bool_compare_and_swap
to force gcc emit inlined instructions.The text was updated successfully, but these errors were encountered: