Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix AArch64InlineAsmReg::emit #131667

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Fix AArch64InlineAsmReg::emit #131667

merged 1 commit into from
Oct 14, 2024

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Oct 13, 2024

Currently, this method uses self as u32 - Self::x0 as u32 to get register index:

rust/compiler/rustc_target/src/asm/aarch64.rs

Line 204 in 3678036

(modifier.unwrap_or('x'), self as u32 - Self::x0 as u32)

However, this is incorrect for reasons explained in the following comment:

rust/compiler/rustc_codegen_llvm/src/asm.rs

Lines 544 to 549 in 3678036

fn a64_reg_index(reg: InlineAsmReg) -> Option<u32> {
use AArch64InlineAsmReg::*;
// Unlike `a64_vreg_index`, we can't subtract `x0` to get the u32 because
// `x19` and `x29` are missing and the integer constants for the
// `x0`..`x30` enum variants don't all match the register number. E.g. the
// integer constant for `x18` is 18, but the constant for `x20` is 19.

r? @Amanieu

@rustbot label O-AArch64 +A-inline-assembly

@rustbot 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. A-inline-assembly Area: Inline assembly (`asm!(…)`) O-AArch64 Armv8-A or later processors in AArch64 mode labels Oct 13, 2024
@taiki-e taiki-e mentioned this pull request Oct 13, 2024
Merged
@Amanieu
Copy link
Member

Amanieu commented Oct 14, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 14, 2024

📌 Commit 67ebb6c has been approved by Amanieu

It is now in the queue for this repository.

@bors 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 Oct 14, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#131667 - taiki-e:aarch64-inline-asm-reg-emi…
71049f0 
…t, r=Amanieu

Fix AArch64InlineAsmReg::emit

Currently, this method uses `self as u32 - Self::x0 as u32` to get register index:
https://github.com/rust-lang/rust/blob/36780360b62320a61e2234b17ec600e8e4785509/compiler/rustc_target/src/asm/aarch64.rs#L204

However, this is incorrect for reasons explained in the following comment:
https://github.com/rust-lang/rust/blob/36780360b62320a61e2234b17ec600e8e4785509/compiler/rustc_codegen_llvm/src/asm.rs#L544-L549

r? `@Amanieu`

`@rustbot` label O-AArch64 +A-inline-assembly
This was referenced Oct 14, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2024
@bors
Auto merge of rust-lang#131690 - matthiaskrgr:rollup-mcau4ol, r=matth…
1d45b2d 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#129424 (Stabilize `Pin::as_deref_mut()`)
 - rust-lang#131332 (Fix clobber_abi and disallow SVE-related registers in Arm64EC inline assembly)
 - rust-lang#131384 (Update precondition tests (especially for zero-size access to null))
 - rust-lang#131430 (Special treatment empty tuple when suggest adding a string literal in format macro.)
 - rust-lang#131550 (Make some tweaks to extern block diagnostics)
 - rust-lang#131667 (Fix AArch64InlineAsmReg::emit)
 - rust-lang#131679 (compiletest: Document various parts of compiletest's `lib.rs`)
 - rust-lang#131682 (Tag PRs affecting compiletest with `A-compiletest`)

Failed merges:

 - rust-lang#131496 (Stabilise `const_make_ascii`.)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2024
@bors
Auto merge of rust-lang#131690 - matthiaskrgr:rollup-mcau4ol, r=matth…
9322d18 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#129424 (Stabilize `Pin::as_deref_mut()`)
 - rust-lang#131332 (Fix clobber_abi and disallow SVE-related registers in Arm64EC inline assembly)
 - rust-lang#131384 (Update precondition tests (especially for zero-size access to null))
 - rust-lang#131430 (Special treatment empty tuple when suggest adding a string literal in format macro.)
 - rust-lang#131550 (Make some tweaks to extern block diagnostics)
 - rust-lang#131667 (Fix AArch64InlineAsmReg::emit)
 - rust-lang#131679 (compiletest: Document various parts of compiletest's `lib.rs`)
 - rust-lang#131682 (Tag PRs affecting compiletest with `A-compiletest`)

Failed merges:

 - rust-lang#131496 (Stabilise `const_make_ascii`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dbb0581 into rust-lang:master Oct 14, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2024
@rust-timer
Unrolled build for rust-lang#131667
dd6dc1b 
Rollup merge of rust-lang#131667 - taiki-e:aarch64-inline-asm-reg-emit, r=Amanieu

Fix AArch64InlineAsmReg::emit

Currently, this method uses `self as u32 - Self::x0 as u32` to get register index:
https://github.com/rust-lang/rust/blob/36780360b62320a61e2234b17ec600e8e4785509/compiler/rustc_target/src/asm/aarch64.rs#L204

However, this is incorrect for reasons explained in the following comment:
https://github.com/rust-lang/rust/blob/36780360b62320a61e2234b17ec600e8e4785509/compiler/rustc_codegen_llvm/src/asm.rs#L544-L549

r? ``@Amanieu``

``@rustbot`` label O-AArch64 +A-inline-assembly
@taiki-e taiki-e deleted the aarch64-inline-asm-reg-emit branch October 14, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) O-AArch64 Armv8-A or later processors in AArch64 mode S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.84.0
Development

Successfully merging this pull request may close these issues.
4 participants
@taiki-e @Amanieu @bors @rustbot