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

Allow clobbering unsupported registers in asm! #83841

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Apr 4, 2021

Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.

cc #81092

r? @nagisa

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 4, 2021
@Amanieu
Copy link
Member Author

Amanieu commented Apr 4, 2021

cc @rust-lang/wg-inline-asm

@rust-log-analyzer

This comment has been minimized.

@Amanieu Amanieu force-pushed the asm_clobber_feature branch 2 times, most recently from fd8b37e to 4d5f999 Compare April 4, 2021 09:03
Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.
let mut layout = None;
let ty = if let Some(ref place) = place {
layout = Some(&place.layout);
llvm_fixup_output_type(self.cx, reg.reg_class(), &place.layout)
} else if !is_target_supported(reg.reg_class()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to happen conditionally? Can't we always "just" clobber the clobber-like constraints, regardless of the target features?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just clobbering loses the distinction between out and lateout: LLVM clobbers act like an out, which prevents the registers from being used as an input. However in the case of lateout we want to allow the register allocator to use the register being clobbered as an input.

For register classes that are unavailable due to disabled target features this is not a problem: the register can't be used as an input anyways.

@nagisa
Copy link
Member

nagisa commented Apr 4, 2021

LGTM r=me, your call what to do with the comments I left inline.

EDIT: could you also add a test case for this comment if there isn't already one while at it? (AFAIU it would end up as a compile-fail ui test.

@Amanieu
Copy link
Member Author

Amanieu commented Apr 4, 2021

EDIT: could you also add a test case for this comment if there isn't already one while at it? (AFAIU it would end up as a compile-fail ui test.

We have an x86 equivalent of this test in src/test/ui/asm/bad-reg.rs.

@nagisa
Copy link
Member

nagisa commented Apr 4, 2021

@bors r+

@nagisa nagisa closed this Apr 4, 2021
@bors
Copy link
Contributor

bors commented Apr 4, 2021

📌 Commit 31d0459 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 Apr 4, 2021
@nagisa nagisa reopened this Apr 4, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 4, 2021
Allow clobbering unsupported registers in asm!

Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.

cc rust-lang#81092

r? `@nagisa`
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2021
Rollup of 7 pull requests

Successful merges:

 - rust-lang#80525 (wasm64 support)
 - rust-lang#83019 (core: disable `ptr::swap_nonoverlapping_one`'s block optimization on SPIR-V.)
 - rust-lang#83717 (rustdoc: Separate filter-empty-string out into its own function)
 - rust-lang#83807 (Tests: Remove redundant `ignore-tidy-linelength` annotations)
 - rust-lang#83815 (ptr::addr_of documentation improvements)
 - rust-lang#83820 (Remove attribute `#[link_args]`)
 - rust-lang#83841 (Allow clobbering unsupported registers in asm!)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f8709ec into rust-lang:master Apr 5, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 5, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants