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

Stabilize asm_goto feature gate #133870

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Dec 4, 2024

Stabilize asm_goto feature (tracked by #119364). The issue will remain open and be updated to track asm_goto_with_outputs.

Reference PR: rust-lang/reference#1693

Stabilization Report

This feature adds a label <block> operand type to asm!. <block> must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the asm! block returns and continues execution.

The block starts a new safety context and unsafe operations within must have additional unsafes; the effect of unsafe that surrounds asm! block is cancelled. See #119364 (comment) and #131544.

It's currently forbidden to use asm_goto with output operands; that is still unstable under asm_goto_with_outputs.

Example:

unsafe {
    asm!(
        "jmp {}",
        label {
            println!("Jumped from asm!");
        }
    );
}

Tests:

  • tests/ui/asm/x86_64/goto.rs
  • tests/ui/asm/x86_64/goto-block-safe.stderr
  • tests/ui/asm/x86_64/bad-options.rs
  • tests/codegen/asm/goto.rs

@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2024

r? @ehuss

rustbot has assigned @ehuss.
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. labels Dec 4, 2024
@nbdd0121
Copy link
Contributor Author

nbdd0121 commented Dec 4, 2024

@rustbot label: -T-compiler +T-lang +A-inline-assembly +F-asm

Cc @rust-lang/wg-inline-asm

@rustbot rustbot added A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 4, 2024
@ehuss
Copy link
Contributor

ehuss commented Dec 4, 2024

r? compiler

cc @Amanieu

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 4, 2024
@rustbot rustbot assigned nnethercote and unassigned ehuss Dec 4, 2024
@ehuss ehuss added I-lang-nominated Nominated for discussion during a lang team meeting. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 4, 2024
@nnethercote
Copy link
Contributor

The code changes look fine for stabilizing this feature, so r=me in terms of code. But I am uncertain about the stabilization process. Is any additional T-lang approval needed before this merges? Has such approval already been obtained?

@nbdd0121
Copy link
Contributor Author

nbdd0121 commented Dec 4, 2024

@nnethercote this would need a FCP.

@jieyouxu
Copy link
Member

jieyouxu commented Dec 5, 2024

r? lang (for T-lang FCP)

@rustbot rustbot assigned tmandry and unassigned nnethercote Dec 5, 2024
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`) I-lang-nominated Nominated for discussion during a lang team meeting. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

6 participants