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

asm: Allow using r9 (ARM) and x18 (AArch64) if they are not reserved by the current target #91643

Merged
merged 2 commits into from
Dec 12, 2021

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Dec 7, 2021

This supersedes #88879.

cc @Skirmisher

r? @joshtriplett

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2021
@Skirmisher
Copy link

Thanks for putting this together! I had meant to reply to your comment on my PR, but it kept slippiing my mind. I'm not familiar with rustc internals at all, so I'm glad you have a solution to the circular dependency problem I ran into.

@bors
Copy link
Contributor

bors commented Dec 9, 2021

☔ The latest upstream changes (presumably #91692) made this pull request unmergeable. Please resolve the merge conflicts.

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 9, 2021
- `r9` on ARM.
- `x18` on AArch64.
- `r0` and `r1` on AVR.
In some cases LLVM will allocate a "reserved register" for `reg` operands even though this register cannot be explicitly specified. Assembly code making use of reserved registers should be careful since `reg` operands may alias with those registers. Reserved registers that can sometimes be allocated are the frame pointer and base pointer in the list above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little light on the explanation.

  • How can I, as a user, "be careful" against register aliasing?
  • If a reserved register is allocated into my registers how can I test for that? How can I fix it from happening again?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the wording here is bad. What I meant to say is that reg may sometimes allocate the frame/base pointer register, even though it is not possible to allocate this register explicitly with "rbp".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In some cases LLVM will allocate a "reserved register" for `reg` operands even though this register cannot be explicitly specified. Assembly code making use of reserved registers should be careful since `reg` operands may alias with those registers. Reserved registers that can sometimes be allocated are the frame pointer and base pointer in the list above.
While `asm!` statements cannot explicitly specify the use of reserved registers (such as frame pointer and base pointer registers), in some cases LLVM will allocate one of these reserved registers for `reg` operands. Assembly code making use of reserved registers should be careful since `reg` operands may use the same registers.

Copy link
Contributor

@Lokathor Lokathor Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That answers one question, but I still don't understand the following:

  • If I type a register class, such as reg, can that give me a register LLVM considers reserved.
  • If so, is that a problem, or will LLVM sort it out somehow?
  • If LLVM doesn't hand that on its own, what is the mitigation that an end programmer can put in their code?

edit: To be clear, I think the answer that Amanieu gave points to it possibly happening but it not being a problem, but the docs themselves should be very clear about whatever the situation is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is actually happening here is that in LLVM the frame pointer and base pointer registers are only marked as reserved if the function requires a frame/base pointer. If a register is not reserved then it may be selected by the register allocator.

In rustc we can't know in advance whether a function will require a frame/base pointer so we always disallow specifying the frame/base pointer as an explicit register. However it could still be selected for reg if the function doesn't need a frame/base pointer.

@Amanieu
Copy link
Member Author

Amanieu commented Dec 10, 2021

I've removed the unstable book changes since that file is going to be deleted by #91728. I'm going to continue working on the wording for reserved registers in rust-lang/reference#1105.

@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 10, 2021

📌 Commit 8716f27 has been approved by joshtriplett

@bors
Copy link
Contributor

bors commented Dec 10, 2021

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Dec 10, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 11, 2021
asm: Allow using r9 (ARM) and x18 (AArch64) if they are not reserved by the current target

This supersedes rust-lang#88879.

cc `@Skirmisher`

r? `@joshtriplett`
@matthiaskrgr
Copy link
Member

Rollup probably related to this pr:

#91788 (comment)

@bors r-

@bors 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 Dec 11, 2021
@Amanieu
Copy link
Member Author

Amanieu commented Dec 11, 2021

@bors r=joshtriplett

@bors
Copy link
Contributor

bors commented Dec 11, 2021

📌 Commit 17766f8 has been approved by joshtriplett

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 11, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2021
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#90081 (Make `intrinsics::write_bytes` const)
 - rust-lang#91643 (asm: Allow using r9 (ARM) and x18 (AArch64) if they are not reserved by the current target)
 - rust-lang#91737 (Make certain panicky stdlib functions behave better under panic_immediate_abort)
 - rust-lang#91750 (rustdoc: Add regression test for Iterator as notable trait on &T)
 - rust-lang#91764 (Do not ICE when suggesting elided lifetimes on non-existent spans.)
 - rust-lang#91780 (Remove hir::Node::hir_id.)
 - rust-lang#91797 (Fix zero-sized reference to deallocated memory)
 - rust-lang#91806 (Make `Unique`s methods `const`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 443ed7c into rust-lang:master Dec 12, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

9 participants