Skip to content

Enable outline-atomics by default on aarch64 Apple targets #149634

Open

Description

@tgross35

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.

This does not affect Darwin or *-sim since LSE is already in the baseline. I have a branch enabling things here https://github.com/tgross35/rust/commit/apple-outline-atomics/.

cc @madsmtm

Activity

  1. added
    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    O-appleOperating system: Apple / Darwin (macOS, iOS, tvOS, visionOS, watchOS)
    on Dec 4, 2025
  2. added
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Dec 4, 2025
  3. removed
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Dec 4, 2025
  4. tgross35 commented on Dec 4, 2025

    @tgross35
    MemberAuthor

    @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?

  5. tgross35 commented on Dec 4, 2025

    @tgross35
    MemberAuthor

    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, w0 instead of uxtb w16, w0 ; adrp x16, HAVE_LSE_ATOMICS; ...

    https://rust.godbolt.org/z/89jj6K8b3 (See the line commented TRYME)

  6. tgross35 commented on Dec 4, 2025

    @tgross35
    MemberAuthor

    If you get the chance to test, tgross35@76a4adc at at least generates the correct assembly.

  7. madsmtm commented on Dec 4, 2025

    @madsmtm
    Member

    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!

    AH! That confused me a lot too!

    If you get the chance to test, tgross35@76a4adc at at least generates the correct assembly.

    I'll try it out, might take me a little while to recall how I tested things before.

  8. madsmtm commented on Dec 4, 2025

    @madsmtm
    Member

    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.

  9. vini-fda commented on Jan 7, 2026

    @vini-fda
    Contributor

    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?

  10. tgross35 commented on Jan 8, 2026

    @tgross35
    MemberAuthor

    Thank you for the interest!

    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.

  11. vini-fda commented on Jan 9, 2026

    @vini-fda
    Contributor

    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.

    Sure, will do! Thanks for the pointers :)

  12. tgross35 commented on Jan 9, 2026

    @tgross35
    MemberAuthor

    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.

  13. added
    A-compiler-builtinsArea: compiler-builtins (https://github.com/rust-lang/compiler-builtins)
    on Jan 11, 2026
  14. thomcc commented on May 11, 2026

    @thomcc
    Member

    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).

  15. tgross35 commented on May 12, 2026

    @tgross35
    MemberAuthor

    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.

  16. thomcc commented on May 12, 2026

    @thomcc
    Member

    Ah, that's fine then. My concern was mostly because had been changed in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.A-compiler-builtinsArea: compiler-builtins (https://github.com/rust-lang/compiler-builtins)O-appleOperating system: Apple / Darwin (macOS, iOS, tvOS, visionOS, watchOS)T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions