Add `-Zfixed-x18` #124655

Darksonn · 2024-05-03T12:36:45Z

This PR is a follow-up to #124323 that proposes a different implementation. Please read the description of that PR for motivation.

See the equivalent flag in the clang docs.

MCP: rust-lang/compiler-team#748
Fixes #121970
r? rust-lang/compiler

Signed-off-by: Alice Ryhl <[email protected]>

estebank · 2024-05-07T22:55:31Z

r? @RalfJung

It seems like the right person to review this? The changes look reasonable to me.

RalfJung · 2024-05-08T05:50:59Z

Sorry, I can't do reviews outside the interpreter.
r? compiler

Darksonn · 2024-05-09T13:22:42Z

@apiraino is an MCP needed?

apiraino · 2024-05-14T10:43:32Z

@apiraino is an MCP needed?

Posting here the answer from compiler team (see Zulip): it's to review this without MCP since it's an unstable flag

compiler/rustc_codegen_llvm/src/llvm_util.rs

Darksonn · 2024-05-15T09:21:37Z

compiler/rustc_codegen_llvm/src/llvm_util.rs

+    // -Zfixed-x18
+    if sess.opts.unstable_opts.fixed_x18 {
+        if sess.target.arch != "aarch64" {
+            sess.dcx().emit_fatal(FixedX18InvalidArch { arch: &sess.target.arch });


I used emit_fatal here because with emit_err, the error is printed twice.

oli-obk · 2024-05-22T04:36:43Z

r? @lqd

lqd · 2024-05-22T17:59:59Z

@oli-obk sure, I can review when the MCP completes if you'd like (I think @estebank already approved)

Darksonn · 2024-05-28T11:06:57Z

The MCP has completed.

lqd · 2024-05-29T14:31:03Z

We probably don't need all the blessed expectations compared to e.g.

//@ error-pattern: the `-Zfixed-x18` flag is not supported
//@ dont-check-compiler-stderr

but it's fine.

bsmith mentioned

cc-rs would need to be updated to forward this option

Does this need to be done soon after merging this PR, or when the full design is finalized for stabilization?

On zulip, you, ralf and bjorn were still discussing details, but it's not clear to me whether these mention possible future work, or an approach you three would rather take in this PR? If it's the former, this looks good enough to land unstably to me, and we can iterate from there if need be.

Darksonn · 2024-05-29T14:43:35Z

cc @NobodyXu on the cc-rs question

On zulip, you, ralf and bjorn were still discussing details, but it's not clear to me whether these mention possible future work, or an approach you three would rather take in this PR?

I would rather land this PR now as unstable and change it later.

lqd · 2024-05-29T16:01:58Z

That seems good to me as well if RfL is fine with the possible churn, as I'd expect you to be the only consumers for this flag for a while.

Thanks!
@bors r=lqd,estebank

bors · 2024-05-29T16:02:00Z

📌 Commit 4aafecb has been approved by lqd,estebank

It is now in the queue for this repository.

ojeda · 2024-05-29T17:17:59Z

That seems good to me as well if RfL is fine with the possible churn, as I'd expect you to be the only consumers for this flag for a while.

It should be fine, and this way we can start getting some usage/testing out of it, including in the potential/upcoming Rust for Linux CI job here (where churn should not matter, in the sense that we can change the job at the same time in the same PR to adapt). If we really expect a lot of churn, we could consider avoiding to put it in upstream Linux just yet, but I think it would still be fine.

Thanks Rémy!

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#124655 (Add `-Zfixed-x18`) - rust-lang#125693 (Format all source files in `tests/coverage/`) - rust-lang#125700 (coverage: Avoid overflow when the MC/DC condition limit is exceeded) - rust-lang#125705 (Reintroduce name resolution check for trying to access locals from an inline const) - rust-lang#125708 (tier 3 target policy: clarify the point about producing assembly) - rust-lang#125715 (remove unneeded extern crate in rmake test) - rust-lang#125719 (Extract coverage-specific code out of `compiletest::runtest`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#124655 - Darksonn:fixed-x18, r=lqd,estebank Add `-Zfixed-x18` This PR is a follow-up to rust-lang#124323 that proposes a different implementation. Please read the description of that PR for motivation. See the equivalent flag in [the clang docs](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-ffixed-x18). MCP: rust-lang/compiler-team#748 Fixes rust-lang#121970 r? rust-lang/compiler

fmease · 2024-05-29T23:11:51Z

bors sleepy @bors r-

NobodyXu · 2024-05-30T06:54:35Z

cc @NobodyXu on the cc-rs question

On zulip, you, ralf and bjorn were still discussing details, but it's not clear to me whether these mention possible future work, or an approach you three would rather take in this PR?

I would rather land this PR now as unstable and change it later.

Do you mean:

That way, cc-rs can translate -Ctarget-feature=-x18-available into -ffixed-x18 as well, instead of emitting -ffixed-x18 when you do nothing.

it sounds alright to me

Darksonn · 2024-05-30T07:18:48Z

@NobodyXu What landed yesterday is a -Zfixed-x18 flag that needs to be converted to -ffixed-x18. Potentially we change the flag to an x18-available feature in the future. Does cc-rs need to support -Zfixed-x18 now?

NobodyXu · 2024-05-30T07:23:33Z

What landed yesterday is a -Zfixed-x18 flag that needs to be converted to -ffixed-x18. Potentially we change the flag to an x18-available feature in the future. Does cc-rs need to support -Zfixed-x18 now?

Thanks for explanation, currently nobody requested for that feature, but it might be a feature good to have just in case someone needs it.

Make `-Zfixed-x18` into a target modifier As part of rust-lang#136966, the `-Zfixed-x18` flag should be turned into a target modifier. This is a blocker to stabilization of the flag. The flag was originally added in rust-lang#124655 and the MCP for its addition is [MCP#748](rust-lang/compiler-team#748). On some aarch64 targets, the x18 register is used as a temporary caller-saved register by default. When the `-Zfixed-x18` flag is passed, this is turned off so that the compiler doesn't use the x18 register. This allows end-users to use the x18 register for other purposes. For example, by accessing it with inline asm you can use the register as a very efficient thread-local variable. Another common use-case is to store the stack pointer needed by the shadow-call-stack sanitizer. There are also some aarch64 targets where not using x18 is the default – in those cases the flag is a no-op. Note that this flag does not *on its own* cause an ABI mismatch. What actually causes an ABI mismatch is when you have different compilation units that *disagree* on what it should be used for. But having a CU that uses it and another CU that doesn't normally isn't enough to trigger an ABI problem. However, we still consider the flag to be a target modifier in all cases, since it is assumed that you are passing the flag because you intend to assign some other meaning to the register. Rejecting all flag mismatches even if not all are unsound is consistent with [RFC#3716](https://rust-lang.github.io/rfcs/3716-target-modifiers.html). See the headings "not all mismatches are unsound" and "cases that are not caught" for additional discussion of this. On aarch64 targets where `-Zfixed-x18` is not a no-op, it is an error to pass `-Zsanitizer=shadow-call-stack` without also passing `-Zfixed-x18`.

Add -Zfixed-x18

40f0172

Signed-off-by: Alice Ryhl <[email protected]>

rustbot assigned estebank May 3, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 3, 2024

Darksonn mentioned this pull request May 3, 2024

Add reserve-x18 target feature for aarch64 #124323

Closed

ojeda mentioned this pull request May 3, 2024

rustc wanted features & bugfixes Rust-for-Linux/linux#355

Open

56 tasks

rustbot assigned RalfJung and unassigned estebank May 7, 2024

estebank approved these changes May 7, 2024

View reviewed changes

rustbot assigned oli-obk and unassigned RalfJung May 8, 2024

Fail on non-aarch64 targets

518becf

Darksonn commented May 14, 2024

View reviewed changes

compiler/rustc_codegen_llvm/src/llvm_util.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Use an error struct instead of a panic

b780fa9

Darksonn commented May 15, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

Remove fixed_x18.aarch64.stderr

0ce51f5

This comment has been minimized.

Sign in to view

Remove aarch64 from revisions list

7677ff2

Darksonn mentioned this pull request May 15, 2024

-Zfixed-x18 rust-lang/compiler-team#748

Closed

3 tasks

rustbot assigned lqd and unassigned oli-obk May 22, 2024

Simplify check for unsupported architectures

4aafecb

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2024

matthiaskrgr mentioned this pull request May 29, 2024

Rollup of 7 pull requests #125732

Merged

bors merged commit d0311c1 into rust-lang:master May 29, 2024
6 checks passed

rustbot added this to the 1.80.0 milestone May 29, 2024

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 29, 2024

ojeda mentioned this pull request May 30, 2024

Rust unstable features needed for the kernel Rust-for-Linux/linux#2

Open

94 tasks

Darksonn deleted the fixed-x18 branch May 30, 2024 07:17

taiki-e mentioned this pull request Nov 25, 2024

Fix handling of x18 in AArch64 inline assembly on ohos/trusty or with -Zfixed-x18 #133463

Merged

Darksonn mentioned this pull request May 5, 2025

Make -Zfixed-x18 into a target modifier #140661

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `-Zfixed-x18` #124655

Add `-Zfixed-x18` #124655

Darksonn commented May 3, 2024 •

edited

Loading

estebank commented May 7, 2024

RalfJung commented May 8, 2024

Darksonn commented May 9, 2024

apiraino commented May 14, 2024 •

edited

Loading

This comment has been minimized.

Darksonn May 15, 2024

This comment has been minimized.

This comment has been minimized.

oli-obk commented May 22, 2024

lqd commented May 22, 2024

Darksonn commented May 28, 2024

lqd commented May 29, 2024

Darksonn commented May 29, 2024

lqd commented May 29, 2024

bors commented May 29, 2024

ojeda commented May 29, 2024 •

edited

Loading

fmease commented May 29, 2024

NobodyXu commented May 30, 2024

Darksonn commented May 30, 2024

NobodyXu commented May 30, 2024

Add -Zfixed-x18 #124655

Add -Zfixed-x18 #124655

Conversation

Darksonn commented May 3, 2024 • edited Loading

estebank commented May 7, 2024

RalfJung commented May 8, 2024

Darksonn commented May 9, 2024

apiraino commented May 14, 2024 • edited Loading

This comment has been minimized.

Darksonn May 15, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

oli-obk commented May 22, 2024

lqd commented May 22, 2024

Darksonn commented May 28, 2024

lqd commented May 29, 2024

Darksonn commented May 29, 2024

lqd commented May 29, 2024

bors commented May 29, 2024

ojeda commented May 29, 2024 • edited Loading

fmease commented May 29, 2024

NobodyXu commented May 30, 2024

Darksonn commented May 30, 2024

NobodyXu commented May 30, 2024

Add `-Zfixed-x18` #124655

Add `-Zfixed-x18` #124655

Darksonn commented May 3, 2024 •

edited

Loading

apiraino commented May 14, 2024 •

edited

Loading

ojeda commented May 29, 2024 •

edited

Loading