Enable `outline-atomics` by default on aarch64 Apple targets

[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

[tgross35]

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

[tgross35]

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)

[tgross35]

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

[madsmtm]

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.

[madsmtm]

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.

[vini-fda]

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?

[tgross35]

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.