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

Add clobber-only register classes for asm! #86416

Merged
merged 2 commits into from
Jul 11, 2021

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jun 17, 2021

These are needed to properly express a function call ABI using a clobber
list, even though we don't support passing actual values into/out of
these registers.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 Jun 17, 2021
@JohnCSimon JohnCSimon added F-rust_2018_preview `#![feature(rust_2018_preview)]` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. F-rust_2018_preview `#![feature(rust_2018_preview)]` labels Jul 6, 2021
@Amanieu
Copy link
Member Author

Amanieu commented Jul 7, 2021

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned matthewjasper Jul 7, 2021
@@ -199,6 +199,19 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
);

// Some register classes can only be used as clobbers.
if reg_class.is_clobber_only(asm_arch.unwrap())
&& (!is_clobber || matches!(reg, asm::InlineAsmRegOrRegClass::RegClass(_)))
Copy link
Member

Choose a reason for hiding this comment

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

I'm not following what purpose the matches!(reg, asm::InlineAsmRegOrRegClass::RegClass(_)) part is serving. Could you explain and/or add a comment?

v28: vreg = ["v28", "b28", "h28", "s28", "d28", "q28", "z28"],
v29: vreg = ["v29", "b29", "h29", "s29", "d29", "q29", "z29"],
v30: vreg = ["v30", "b30", "h30", "s30", "d30", "q30", "z30"],
v31: vreg = ["v31", "b31", "h31", "s31", "d31", "q31", "z31"],
Copy link
Member

Choose a reason for hiding this comment

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

If I understand this correctly, the addition of z registers is not strictly related to the clobber-only register classes, correct? While related to the p registers, I don't believe p registers are required to utilize the z registers, right?

If that's the case, could this become a separate commit?

@nagisa nagisa 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2021
Amanieu added 2 commits July 10, 2021 17:29
These are needed to properly express a function call ABI using a clobber
list, even though we don't support passing actual values into/out of
these registers.
@Amanieu Amanieu force-pushed the asm_clobber_only branch from 5ede032 to d2a1d04 Compare July 10, 2021 15:29
@Amanieu Amanieu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2021
@nagisa
Copy link
Member

nagisa commented Jul 10, 2021

@bors r+ Thanks!

@bors
Copy link
Contributor

bors commented Jul 10, 2021

📌 Commit d2a1d04 has been approved by nagisa

@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 Jul 10, 2021
@bors
Copy link
Contributor

bors commented Jul 11, 2021

⌛ Testing commit d2a1d04 with merge 99f8efe...

@bors
Copy link
Contributor

bors commented Jul 11, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 99f8efe to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 11, 2021
@bors bors merged commit 99f8efe into rust-lang:master Jul 11, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants