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.

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

adjust documented inline-asm register constraints #90736

Merged
merged 3 commits into from
Nov 11, 2021
Merged

adjust documented inline-asm register constraints #90736

merged 3 commits into from
Nov 11, 2021

Conversation

Lokathor
Copy link
Contributor

@Lokathor Lokathor commented Nov 9, 2021

This change more clearly specifies how reg and reg_thumb work with ARM, Thumb2, and Thumb1 code.

Based upon the llvm documentation for register constraint codes.
To be clear, this just updates the docs to match what already happens with rustc/llvm.
No change in the compiler is required to make it match this new documentation.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 9, 2021
@Lokathor
Copy link
Contributor Author

Lokathor commented Nov 9, 2021

(as discussed at the 2021-11-09 T-lang meeting)

cc @Amanieu and @joshtriplett

@Lokathor
Copy link
Contributor Author

Lokathor commented Nov 9, 2021

It's my understanding that Amaneau would prefer that the reg class not do separate things on thumb1 as compared to thumb2 and ARM, but LLVM already operates this way. If the full range of registers is allowed on thumb1 then it just leads to strange compile errors. Registers above r0-r7 aren't allowed in thumb1 with instructions other than add, mov, and cmp, so those registers shouldn't be allocated to thumb1 inline assembly by default.

If we want to allow the high registers to be used with thumb1 inline assembly, a separate register class could be made for that, but note that LLVM doesn't even actually support that use case at this time. There is not any register class you can provide to LLVM to make give you the high registers (r8 and above) with thumb1 code.

@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the PR/issue. A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) labels Nov 9, 2021
Lokathor and others added 2 commits November 9, 2021 15:52
Co-authored-by: Josh Triplett <josh@joshtriplett.org>
Co-authored-by: Josh Triplett <josh@joshtriplett.org>
@Amanieu
Copy link
Member

Amanieu commented Nov 10, 2021

I'm actually starting to have second thoughts on the value of having a separate reg_thumb register class. I'm probably going to remove it before stabilization.

In any case, this PR accurately describes what currently happens.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 10, 2021

📌 Commit a306d35 has been approved by Amanieu

@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 Nov 10, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 10, 2021
…askrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#89930 (Only use `clone3` when needed for pidfd)
 - rust-lang#90736 (adjust documented inline-asm register constraints)
 - rust-lang#90783 (Update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 90bb5fc into rust-lang:master Nov 11, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 11, 2021
Amanieu added a commit to Amanieu/rust that referenced this pull request Dec 7, 2021
Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2021
…plett

Remove the reg_thumb register class for asm! on ARM

Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736.

cc `@Lokathor`

r? `@joshtriplett`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2021
…plett

Remove the reg_thumb register class for asm! on ARM

Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736.

cc ``@Lokathor``

r? ``@joshtriplett``
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Dec 31, 2021
Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736.
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Dec 31, 2021
…plett

Remove the reg_thumb register class for asm! on ARM

Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736.

cc ``@Lokathor``

r? ``@joshtriplett``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants