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

Fix asm goto with outputs and move it to a separate feature gate #131523

Merged
merged 3 commits into from
Nov 25, 2024

Conversation

nbdd0121
Copy link
Contributor

Tracking issue: #119364

This PR addresses 3 aspects of asm goto with outputs:

  • Codegen is fixed. My initial implementation has an oversight which cause the output to be only stored in fallthrough path, but not in label blocks.
  • Outputs can now be used with options(noreturn) if a label block is given.
  • All of this is moved to a new feature gate, because we likely want to stabilise asm_goto before asm goto with outputs.

@rustbot labels: +A-inline-assembly +F-asm

@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) labels Oct 11, 2024
@bors
Copy link
Contributor

bors commented Oct 23, 2024

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

@compiler-errors
Copy link
Member

r? compiler-errors

I'll review this, please ping me if I don't get to it in a few days

@rustbot rustbot assigned compiler-errors and unassigned TaKO8Ki Oct 28, 2024
@bors
Copy link
Contributor

bors commented Nov 2, 2024

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

@nbdd0121
Copy link
Contributor Author

ping @compiler-errors

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

One nit then r=me

let mut _out: u64;
unsafe {
asm!("mov {}, 1", "jmp {}", out(reg) _out, label {});
//~^ ERROR using both label and outputs operands for inline assembly is unstable
Copy link
Member

Choose a reason for hiding this comment

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

Is this a typo? I think it should be "label and output operands"

@compiler-errors
Copy link
Member

@bors delegate+

@bors
Copy link
Contributor

bors commented Nov 24, 2024

✌️ @nbdd0121, you can now approve this pull request!

If @compiler-errors told you to "r=me" after making some further change, please make that change, then do @bors r=@compiler-errors

@compiler-errors
Copy link
Member

You also may want to rebase this PR just in case

@compiler-errors compiler-errors 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 Nov 24, 2024
When outputs are used together with labels, they are considered
to be written for all destinations, not only when falling through.
When labels are present, the `noreturn` option really means that asm block
won't fallthrough -- if labels are present, then outputs can still be
meaningfully used.
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 24, 2024

📌 Commit 0178ba2 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 24, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 24, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 25, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#131523 (Fix asm goto with outputs and move it to a separate feature gate)
 - rust-lang#131664 (Support input/output in vector registers of s390x inline assembly (under asm_experimental_reg feature))
 - rust-lang#132432 (Add a test to verify that libstd doesn't use protected symbols)
 - rust-lang#132502 (Document possibility to set core features in example config.toml)
 - rust-lang#132529 (ci(triagebot): add more top-level files to A-meta)
 - rust-lang#132533 (Add BorrowedBuf::into_filled{,_mut} methods to allow returning buffer with original lifetime)
 - rust-lang#132803 (Fix broken url)
 - rust-lang#132982 (alloc: fix `Allocator` method names in `alloc` free function docs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c5230d1 into rust-lang:master Nov 25, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Nov 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 25, 2024
Rollup merge of rust-lang#131523 - nbdd0121:asm, r=compiler-errors

Fix asm goto with outputs and move it to a separate feature gate

Tracking issue: rust-lang#119364

This PR addresses 3 aspects of asm goto with outputs:
* Codegen is fixed. My initial implementation has an oversight which cause the output to be only stored in fallthrough path, but not in label blocks.
* Outputs can now be used with `options(noreturn)` if a label block is given.
* All of this is moved to a new feature gate, because we likely want to stabilise `asm_goto` before asm goto with outputs.

`@rustbot` labels: +A-inline-assembly +F-asm
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-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.

5 participants