Remove +outline-atomics from arm64 linux

[jyn514]

Proposal

Remove -C target-feature=+outline-atomics from the default target spec for aarch64-unknown-linux-gnu . In particular, the following targets would be affected:

; rg outline-atomics compiler/rustc_target
compiler/rustc_target/src/spec/aarch64_be_unknown_linux_gnu.rs
11:            features: "+v8a,+outline-atomics".into(),

compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu_ilp32.rs
11:            features: "+v8a,+outline-atomics".into(),

compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu.rs
10:            features: "+v8a,+outline-atomics".into(),

compiler/rustc_target/src/spec/aarch64_be_unknown_linux_gnu_ilp32.rs
15:            features: "+v8a,+outline-atomics".into(),

See rust-lang/rust#109064 for a detailed writeup of why I want to do this; tl;dr it introduces a hard dependency on LLVM's compiler-rt, which means that the compiler_builtins crate is broken in dev builds of the compiler itself, as well as when using cargo -Zbuild-std. The only downside of this change I can see is that it makes atomics slightly slower on aarch64.

Alternatives

• Do nothing; keep outline-atomics on aarch64.
• Keep the feature and emulate it in compiler_builtins without libc. This looks tricky but not impossible; +outline-atomics violates "core shall not depend on libc" rust#109064 (comment) has suggestions. Unfortunately it appears to not be possible on all targets, so this will also hurt portability: +outline-atomics violates "core shall not depend on libc" rust#109064 (comment)
• Keep the feature and emulate it in compiler_builtins, using the target libc. This will keep the dependency on libc for the target, but will solve most of the pain for build-std and x check --target aarch64-unknown-linux-gnu.

Mentors or Reviewers

@workingjubilee, possibly @thomcc

Process

The main points of the Major Change Process are as follows:

• File an issue describing the proposal.
• A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

[jyn514]

@Mark-Simulacrum said:

The original PR introducing outline-atomics said:

Performance improves by several orders of magnitude when using the LSE instructions
instead of the ARMv8.0 compatible load/store exclusive instructions.

That feels a little bit different from the claim in this MCP that atomic are just "slightly slower on aarch64".

@jyn514 said:

fwiw we don't enable this feature on any target other than aarch64 linux so it seems a little strange to hold it to a higher standard wrt perf than other targets ... at least we should be consistent about enabling it

@Mark-Simulacrum said:

I admit I don't know what other targets are like, but naively I was assuming they don't have a comparable performance delta available.

@sebpop I can't find your Zulip username - what was the original motivation for choosing specifically aarch64-linux for this feature? Would it have a comparable perf improvement on other targets?

[jyn514]

I am no longer planning to completely remove this feature on aarch64-linux. Instead I am adding a fallback version of these intrinsics to compiler-builtins so that we don't have a hard dependency on libc and compiler-rt: rust-lang/compiler-builtins#532