You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Enable outline-atomics by default on aarch64 Apple targets #149634
This was originally part of #144938, but per #144938 (review) this does not seem to work correctly. Seems like there may be some behavior to clarify with LLVM, or a possible bug there.
@madsmtm quoting your comment that's linked above:
I verified that __rust_enable_lse is run on Apple platforms when having -lse,+outline-atomics.
It isn't actually used though, the code will always fall back to the slower, non LSE implementation (regardless of whether the current CPU actually supports it), since try_lse_op! is gated with .arch_extension lse, which doesn't seem to be enabled on Apple platforms?
And if I just remove it, I get:
error: ADR/ADRP relocations must be GOT relative
|
note: instantiated into assembly here
--> <inline asm>:6:14
|
6 | adrp x16, __ZN17compiler_builtins23aarch64_outline_atomics16HAVE_LSE_ATOMICS17h7f004a93068845c5E; nop; ldrb w16, [x16, :lo12:__ZN17compiler_builtins...
| ^
...
I believe .arch_extension lse shouldn't have any runtime influence, it should just tell be telling the assembler that LSE instructions are allowed in the block. E.g. https://rust.godbolt.org/z/hfqxf78bz works fine but commenting the directive gets "error: instruction requires: lse".
So any chance you remember a bit more here or would be able to rerun?
I tried it by setting features: "+outline-atomics,-lse".into(), in aarch64_apple_darwin.rs, with that, stepping through your example code hits both __rust_enable_lse and __aarch64_cas4_acq, and hits the expected casa w0, w1, [x2] instruction inside __aarch64_cas4_acq.
Hi! I'm willing to contribute to this issue (and issue #150455) if necessary. I read up on the previous threads and I tested it locally on my laptop with the example code from the link provided by @madsmtm, and it works on my machine (M4 mac).
@tgross35 I took the liberty of opening a draft PR with your changes, but I'd imagine it would be interesting to add a few extra tests. I was taking a look at library/compiler-builtins/builtins-test/tests/lse.rs and saw there are some tests for the compare-and-swap instruction, but I'll have to spend some time to wrap my head around what's exactly being tested there (and what's missing). If you have any pointers please feel free to let me know.
Edit: I also noticed that aarch64-apple jobs are not enabled in CI checks for PRs. Is there a reason why?
Edit: I also noticed that aarch64-apple jobs are not enabled in CI checks for PRs. Is there a reason why?
Our full CI has something like 160 hours of CPU time, we can't run that on every PR :)
I'd recommend making the asm changes against https://github.com/rust-lang/compiler-builtins, which is where the subtree lives. CI there is probably more of what you're expecting, I also think c-b tests still don't get run in r-l/rust. After that merges and gets synced through, it will be easy to update the target files.
Our full CI has something like 160 hours of CPU time, we can't run that on every PR :)
Makes sense. So from what I understand the aarch64-apple checks are run only before merging to main, correct? Or are automated tests for apple just not executed when merging PRs?
I'd recommend making the asm changes against https://github.com/rust-lang/compiler-builtins, which is where the subtree lives. CI there is probably more of what you're expecting, I also think c-b tests still don't get run in r-l/rust. After that merges and gets synced through, it will be easy to update the target files.
Our full CI has something like 160 hours of CPU time, we can't run that on every PR :)
Makes sense. So from what I understand the aarch64-apple checks are run only before merging to main, correct? Or are automated tests for apple just not executed when merging PRs?
That's right! We require always-green CI, so main only gets advanced if the full suite passes. Once PRs get approved they go into a queue where they either get tested+merged on their own, or "rolled up" to test+merge in a batch.
We can run specific tests from the full suite on specific PRs, but there currently isn't anything that would actually get tested in the PR because I never got around to wrapping up #142106.
I came across this recently by chance. Why would we do this on aarch64-apple-darwin? M1 and later all have LSE, do they not? (I just checked and even my very old baseline M1 non-max non-pro from right after the M1 came out has both LSE and LSE2).
Why would we turn that off and use outline-atomics (which is slower than native LSE use, and only faster when the target doesn't statically have LSE, but the actual machine dynamically does).
Ah sorry for the confusion here, like I mentioned in the top post Darwin and sim targets aren't intended to change. We've just been testing with it since it's the easiest target.
Activity
tgross35 commentedon Dec 4, 2025
@madsmtm quoting your comment that's linked above:
I believe
.arch_extension lseshouldn't have any runtime influence, it should just tell be telling the assembler that LSE instructions are allowed in the block. E.g. https://rust.godbolt.org/z/hfqxf78bz works fine but commenting the directive gets "error: instruction requires: lse".So any chance you remember a bit more here or would be able to rerun?
tgross35 commentedon Dec 4, 2025
Oh wow, serious wtf moment: the source uses semicolons as line separators within a string https://github.com/rust-lang/compiler-builtins/blob/acb3a0074d8fd8d9ca655a8cddef82decf0aa50d/compiler-builtins/src/aarch64_linux.rs#L142-L145. On most platforms, these turn into newlines. On Apple, these turn into comments!
So the reason LSE wasn't working is because it was executing only
uxtb w16, w0instead ofuxtb w16, w0 ; adrp x16, HAVE_LSE_ATOMICS; ...https://rust.godbolt.org/z/89jj6K8b3 (See the line commented TRYME)
tgross35 commentedon Dec 4, 2025
If you get the chance to test, tgross35@76a4adc at at least generates the correct assembly.
madsmtm commentedon Dec 4, 2025
AH! That confused me a lot too!
I'll try it out, might take me a little while to recall how I tested things before.
madsmtm commentedon Dec 4, 2025
I tried it by setting
features: "+outline-atomics,-lse".into(),inaarch64_apple_darwin.rs, with that, stepping through your example code hits both__rust_enable_lseand__aarch64_cas4_acq, and hits the expectedcasa w0, w1, [x2]instruction inside__aarch64_cas4_acq.vini-fda commentedon Jan 7, 2026
Hi! I'm willing to contribute to this issue (and issue #150455) if necessary. I read up on the previous threads and I tested it locally on my laptop with the example code from the link provided by @madsmtm, and it works on my machine (M4 mac).
@tgross35 I took the liberty of opening a draft PR with your changes, but I'd imagine it would be interesting to add a few extra tests. I was taking a look at
library/compiler-builtins/builtins-test/tests/lse.rsand saw there are some tests for the compare-and-swap instruction, but I'll have to spend some time to wrap my head around what's exactly being tested there (and what's missing). If you have any pointers please feel free to let me know.Edit: I also noticed that aarch64-apple jobs are not enabled in CI checks for PRs. Is there a reason why?
tgross35 commentedon Jan 8, 2026
Thank you for the interest!
Our full CI has something like 160 hours of CPU time, we can't run that on every PR :)
I'd recommend making the asm changes against https://github.com/rust-lang/compiler-builtins, which is where the subtree lives. CI there is probably more of what you're expecting, I also think c-b tests still don't get run in r-l/rust. After that merges and gets synced through, it will be easy to update the target files.
vini-fda commentedon Jan 9, 2026
Makes sense. So from what I understand the aarch64-apple checks are run only before merging to main, correct? Or are automated tests for apple just not executed when merging PRs?
Sure, will do! Thanks for the pointers :)
tgross35 commentedon Jan 9, 2026
That's right! We require always-green CI, so
mainonly gets advanced if the full suite passes. Once PRs get approved they go into a queue where they either get tested+merged on their own, or "rolled up" to test+merge in a batch.We can run specific tests from the full suite on specific PRs, but there currently isn't anything that would actually get tested in the PR because I never got around to wrapping up #142106.
thomcc commentedon May 11, 2026
I came across this recently by chance. Why would we do this on aarch64-apple-darwin? M1 and later all have LSE, do they not? (I just checked and even my very old baseline M1 non-max non-pro from right after the M1 came out has both LSE and LSE2).
Why would we turn that off and use outline-atomics (which is slower than native LSE use, and only faster when the target doesn't statically have LSE, but the actual machine dynamically does).
tgross35 commentedon May 12, 2026
Ah sorry for the confusion here, like I mentioned in the top post Darwin and sim targets aren't intended to change. We've just been testing with it since it's the easiest target.
thomcc commentedon May 12, 2026
Ah, that's fine then. My concern was mostly because had been changed in the PR.